| Message ID | 20221223085047.94832-1-agraf@csgraf.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/intc/arm_gicv3: Bump ITT entry size to 16 | expand |
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