QEMU

Bug #1892540
Comment #13

Comment 13 for bug 1892540

Revision history for this message

Philippe Mathieu-Daudé (philmd) wrote on 2020-10-24: Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter

#13

On 8/30/20 9:32 AM, Mark Cave-Ayland wrote:
> On 29/08/2020 17:45, Philippe Mathieu-Daudé wrote:
>
>> Le sam. 29 août 2020 18:14, Michael <<email address hidden>
>> <mailto:<email address hidden>>> a écrit :
>>
>> Hello,
>>
>> since I wrote the NetBSD code in question, here are my 2 cent:
>>
>> On Sat, 29 Aug 2020 08:41:43 -0700
>> Richard Henderson <<email address hidden>
>> <mailto:<email address hidden>>> wrote:
>>
>> > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
>> > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
>>
>> I don't have it either, but someone did a lot of reverse engineering
>> and gave me his notes. The hardware isn't that complicated, but quite
>> weird.
>>
>> > > However the NetBSD revision 1.32 of the driver introduced
>> > > 64-bit accesses to the stippler and blitter [2]. It is safe
>> > > to assume these memory regions are 64-bit accessible.
>> > > QEMU implementation is 32-bit, so fill the 'impl' fields.
>>
>> IIRC the real hardware *requires* 64bit accesses for stipple and
>> blitter operations to work. For stipples you write a 64bit word into
>> STIP space, the address defines where in the framebuffer you want to
>> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>> BLIT space works similarly, the 64bit word contains an offset were to
>> read pixels from, and how many you want to copy.
>>
>>
>> Thanks Michael for this information!
>> If you don't mind I'll amend it to the commit description so there is a reference for
>> posterity.
>>
>> I'm waiting for /Andreas Gustafsson to test it then will repost.
>
> Hi Philippe,
>
> Thanks for coming up with this patch! Looks fine to me, just wondering if it should
> have a "Fixes: 5d971f9e67 ("memory: Revert "memory: accept mismatching sizes in
> memory_region_access_valid"") tag rather than the original commit since that's how
> other bugs exposed by that commit have been tagged?

I don't think so, the bug was present (hidden) *before* 5d971f9e67 and
we were incorrectly modelling it. I just posted a v3 including Michael
valuable memories :)

>
>
> ATB,
>
> Mark.
>

On 8/30/20 9:32 AM, Mark Cave-Ayland wrote:
> On 29/08/2020 17:45, Philippe Mathieu-Daudé wrote:
> 
>> Le sam. 29 août 2020 18:14, Michael <macallan1888@gmail.com
>> <mailto:macallan1888@gmail.com>> a écrit :
>>
>>      Hello,
>>
>>      since I wrote the NetBSD code in question, here are my 2 cent:
>>
>>      On Sat, 29 Aug 2020 08:41:43 -0700
>>      Richard Henderson <richard.henderson@linaro.org
>>      <mailto:richard.henderson@linaro.org>> wrote:
>>
>>      > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
>>      > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
>>
>>      I don't have it either, but someone did a lot of reverse engineering
>>      and gave me his notes. The hardware isn't that complicated, but quite
>>      weird.
>>
>>      > > However the NetBSD revision 1.32 of the driver introduced
>>      > > 64-bit accesses to the stippler and blitter [2]. It is safe
>>      > > to assume these memory regions are 64-bit accessible.
>>      > > QEMU implementation is 32-bit, so fill the 'impl' fields.
>>
>>      IIRC the real hardware *requires* 64bit accesses for stipple and
>>      blitter operations to work. For stipples you write a 64bit word into
>>      STIP space, the address defines where in the framebuffer you want to
>>      draw, the data contain a 32bit bitmask, foreground colour and a ROP.
>>      BLIT space works similarly, the 64bit word contains an offset were to
>>      read pixels from, and how many you want to copy.
>>
>>
>> Thanks Michael for this information!
>> If you don't mind I'll amend it to the commit description so there is a reference for
>> posterity.
>>
>> I'm waiting for /Andreas Gustafsson to test it then will repost.
> 
> Hi Philippe,
> 
> Thanks for coming up with this patch! Looks fine to me, just wondering if it should
> have a "Fixes: 5d971f9e67 ("memory: Revert "memory: accept mismatching sizes in
> memory_region_access_valid"") tag rather than the original commit since that's how
> other bugs exposed by that commit have been tagged?

I don't think so, the bug was present (hidden) *before* 5d971f9e67 and
we were incorrectly modelling it. I just posted a v3 including Michael
valuable memories :)

> 
> 
> ATB,
> 
> Mark.
>