[0/2] hw/intc/arm_gicv3: Bump ITT entry size to 16

While trying to make Windows work with GICv3 emulation, I stumbled over
the fact that it only supports ITT entry sizes that are power of 2 sized.

While the spec allows arbitrary sizes, in practice hardware will always
expose power of 2 sizes and so this limitation is not really a problem
in real world scenarios. However, we only expose a 12 byte ITT entry size
which makes Windows blue screen on boot.

The easy way to get around that problem is to bump the size to 16. That
is a power of 2, basically is what hardware would expose given the amount
of bits we need per entry and doesn't break any existing scenarios. To
play it safe, this patch set only bumps them on newer machine types.

Alexander Graf (2):
  hw/intc/arm_gicv3: Make ITT entry size configurable
  hw/intc/arm_gicv3: Bump ITT entry size to 16

 hw/core/machine.c                      |  4 +++-
 hw/intc/arm_gicv3_its.c                | 13 ++++++++++---
 hw/intc/gicv3_internal.h               |  2 +-
 include/hw/intc/arm_gicv3_its_common.h |  1 +
 4 files changed, 15 insertions(+), 5 deletions(-)

On 23/12/22 09:50, Alexander Graf wrote:
> While trying to make Windows work with GICv3 emulation, I stumbled over
> the fact that it only supports ITT entry sizes that are power of 2 sized.
> 
> While the spec allows arbitrary sizes, in practice hardware will always
> expose power of 2 sizes and so this limitation is not really a problem
> in real world scenarios. However, we only expose a 12 byte ITT entry size
> which makes Windows blue screen on boot.
> 
> The easy way to get around that problem is to bump the size to 16. That
> is a power of 2, basically is what hardware would expose given the amount
> of bits we need per entry and doesn't break any existing scenarios. To
> play it safe, this patch set only bumps them on newer machine types.
> 
> Alexander Graf (2):
>    hw/intc/arm_gicv3: Make ITT entry size configurable
>    hw/intc/arm_gicv3: Bump ITT entry size to 16

Series:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

On Fri, 23 Dec 2022 at 08:50, Alexander Graf <agraf@csgraf.de> wrote:
>
> While trying to make Windows work with GICv3 emulation, I stumbled over
> the fact that it only supports ITT entry sizes that are power of 2 sized.
>
> While the spec allows arbitrary sizes, in practice hardware will always
> expose power of 2 sizes and so this limitation is not really a problem
> in real world scenarios. However, we only expose a 12 byte ITT entry size
> which makes Windows blue screen on boot.
>
> The easy way to get around that problem is to bump the size to 16. That
> is a power of 2, basically is what hardware would expose given the amount
> of bits we need per entry and doesn't break any existing scenarios. To
> play it safe, this patch set only bumps them on newer machine types.

This is a Windows bug and should IMHO be fixed in that guest OS.
Changing the ITT entry size of QEMU's implementation introduces
an unnecessary incompatibility in migration and wastes memory
(we're already a bit unnecessarily profligate with ITT entries
compared to real hardware).

thanks
-- PMM

Hi Peter,

On 03.01.23 18:41, Peter Maydell wrote:
> On Fri, 23 Dec 2022 at 08:50, Alexander Graf <agraf@csgraf.de> wrote:
>> While trying to make Windows work with GICv3 emulation, I stumbled over
>> the fact that it only supports ITT entry sizes that are power of 2 sized.
>>
>> While the spec allows arbitrary sizes, in practice hardware will always
>> expose power of 2 sizes and so this limitation is not really a problem
>> in real world scenarios. However, we only expose a 12 byte ITT entry size
>> which makes Windows blue screen on boot.
>>
>> The easy way to get around that problem is to bump the size to 16. That
>> is a power of 2, basically is what hardware would expose given the amount
>> of bits we need per entry and doesn't break any existing scenarios. To
>> play it safe, this patch set only bumps them on newer machine types.
> This is a Windows bug and should IMHO be fixed in that guest OS.


I don't have access to the Windows source code, but the compiled binary 
very explicitly checks and validates that an ITT entry is Po2 sized. 
That means the MS folks deliberately decided to make simplifying 
assumptions that hardware will never use any other sizes.

After thinking about it for a while, I ended up with the same 
conclusion: Hardware would never use anything but Po2 sizes because 
those are trivial to map to indexes in hardware, while anything even 
remotely odd is much more costly (in die space and/or time) to extract 
an index from.

So while I'm really curious about the rationale they had here, I doubt 
it's a bug. It's a deliberate decision. And one that makes sense in the 
context of hardware. I don't see a good reason for them to change the 
behavior, given that there's a close-to-0 chance we will ever see real 
hardware ITS structures with ITT entries that are not Po2 sized.


> Changing the ITT entry size of QEMU's implementation introduces
> an unnecessary incompatibility in migration and wastes memory

The patch set deals with migration through machine versions. We do these 
type of changes all the time, why would it be a problem here?

As for memory waste, I agree. If I understand the ITS code correctly, 
basically all of the contents that are >8 bytes is GICv4 related and 
useless in a GICv3 vGIC. So I think if we really care strongly about 
memory waste, we could try to condense it down to 8 bytes in the GICv3 
case and make it 16 only for GICv4.

I think keeping GICv3 and GICv4 code paths identical does have its 
attractiveness though, so I'd prefer not to do it.


> (we're already a bit unnecessarily profligate with ITT entries
> compared to real hardware).

Do you mean the number of entries or the size per entry?


Alex

On 03.01.23 18:41, Peter Maydell wrote:
> On Fri, 23 Dec 2022 at 08:50, Alexander Graf <agraf@csgraf.de> wrote:
>> While trying to make Windows work with GICv3 emulation, I stumbled over
>> the fact that it only supports ITT entry sizes that are power of 2 sized.
>>
>> While the spec allows arbitrary sizes, in practice hardware will always
>> expose power of 2 sizes and so this limitation is not really a problem
>> in real world scenarios. However, we only expose a 12 byte ITT entry size
>> which makes Windows blue screen on boot.
>>
>> The easy way to get around that problem is to bump the size to 16. That
>> is a power of 2, basically is what hardware would expose given the amount
>> of bits we need per entry and doesn't break any existing scenarios. To
>> play it safe, this patch set only bumps them on newer machine types.
> This is a Windows bug and should IMHO be fixed in that guest OS.
> Changing the ITT entry size of QEMU's implementation introduces
> an unnecessary incompatibility in migration and wastes memory
> (we're already a bit unnecessarily profligate with ITT entries
> compared to real hardware).


Follow-up on this: Microsoft has fixed the issue in Windows. That won't 
make older versions work, but the current should be fine with GICv3:

https://fosstodon.org/@itanium/109909281184181276


Alex