| Message ID | 20241105155800.5461-1-phil@philjordan.eu (mailing list archive) |
|---|---|
| Headers | show |
| Series | i386/hvf: x2apic support and some small fixes | expand |
On Tue, Nov 05, 2024 at 04:57:55PM +0100, Phil Dennis-Jordan wrote: > This is a loose collection of patches against the x86 hvf accel. They > can be applied/pulled independently from one another. > > Patch 1 is a repost of a patch I've submitted a bunch of times already. > It wires up and enables x2APIC mode in conjunction with HVF - the > software APIC implementation in QEMU gained the feature earlier this > year but hvf wasn't included. > The change typically improves performance with modern SMP guest OSes by > a 2-digit percentage. (Exact values depend on workload.) > > Patch 2 fixes cases of undefined behaviour recently introduced by commit > 7cac7aa which made changes to HVF CPUID XSAVE functionality. > > Patch 3 fixes a minor one-off memory leak during hvf startup. > > Patch 4 ever so slightly improves APIC correctness under hvf: when > setting the APICBASE MSR, if the APIC deems the new value invalid, > we raise an exception (as per spec) rather than silently doing > nothing. This fixes a failing kvm-unit-tests test case. > > Patch 5 removes some unnecessary duplication and type-rechecking in > HVF's inner loop. (No need to cast the cpu state pointer to X86CPU > within, the hvf_vcp_exec function already does that once at the top.) > > Some of this work has been sponsored by Sauce Labs Inc. > > Phil Dennis-Jordan (5): > i386/hvf: Integrates x2APIC support with hvf accel > i386/hvf: Fix for UB in handling CPUID function 0xD > i386/hvf: Fixes startup memory leak (vmcs caps) > i386/hvf: Raise exception on error setting APICBASE > i386/hvf: Removes duplicate/shadowed variables in hvf_vcpu_exec > To the series, Reviewed-by: Roman Bolshakov <rbolshakov@ddn.com> Tested-by: Roman Bolshakov <rbolshakov@ddn.com> I figured the issue with 24.04 guests, it was an issue on my side (too little memory provided to the guest). Paolo, please apply this if you have no objections. Regards, Roman
Hi Roman, Welcome back, and thanks for reviewing these patches! On Wed, 6 Nov 2024 at 16:26, Roman Bolshakov <roman@roolebo.dev> wrote: > On Tue, Nov 05, 2024 at 04:57:55PM +0100, Phil Dennis-Jordan wrote: > > This is a loose collection of patches against the x86 hvf accel. They > > can be applied/pulled independently from one another. > > > > Patch 1 is a repost of a patch I've submitted a bunch of times already. > > It wires up and enables x2APIC mode in conjunction with HVF - the > > software APIC implementation in QEMU gained the feature earlier this > > year but hvf wasn't included. > > The change typically improves performance with modern SMP guest OSes by > > a 2-digit percentage. (Exact values depend on workload.) > > > > Patch 2 fixes cases of undefined behaviour recently introduced by commit > > 7cac7aa which made changes to HVF CPUID XSAVE functionality. > > > > Patch 3 fixes a minor one-off memory leak during hvf startup. > > > > Patch 4 ever so slightly improves APIC correctness under hvf: when > > setting the APICBASE MSR, if the APIC deems the new value invalid, > > we raise an exception (as per spec) rather than silently doing > > nothing. This fixes a failing kvm-unit-tests test case. > > > > Patch 5 removes some unnecessary duplication and type-rechecking in > > HVF's inner loop. (No need to cast the cpu state pointer to X86CPU > > within, the hvf_vcp_exec function already does that once at the top.) > > > > Some of this work has been sponsored by Sauce Labs Inc. > > > > Phil Dennis-Jordan (5): > > i386/hvf: Integrates x2APIC support with hvf accel > > i386/hvf: Fix for UB in handling CPUID function 0xD > > i386/hvf: Fixes startup memory leak (vmcs caps) > > i386/hvf: Raise exception on error setting APICBASE > > i386/hvf: Removes duplicate/shadowed variables in hvf_vcpu_exec > > > > To the series, > Reviewed-by: Roman Bolshakov <rbolshakov@ddn.com> > Tested-by: Roman Bolshakov <rbolshakov@ddn.com> > > I figured the issue with 24.04 guests, it was an issue on my side (too > little memory provided to the guest). > I'm glad it was that simple - I'm not aware of any incompatibilities, and I did test with a bunch of guest OSes including a few Linux versions, FreeBSD, Win10, and macOS. Regarding your question about kvm-unit-tests: Last time I checked, the upstream version of its APIC tests crashed Qemu on hvf (and I think also TCG,) with or without any of the patches in this patch set applied. The reason is that one of the test cases assumes it's running on a hypervisor supporting a KVM-specific hypercall, i.e. it assumes it's running on KVM. I tried submitting a patch that tests for support of that hypercall at runtime, but it didn't get merged at the time: https://marc.info/?l=kvm&m=169736751712535&w=2 I should update that to make sure it applies correctly to the latest upstream and re-submit it, as those unit tests are actually extremely useful for development & debugging. If the patch no longer applies correctly, you can also just disable the test_pv_ipi() test by whatever means you like. Patch 1/5 obviously(?) allows the x2apic tests to run at all, while patch 4/5 fixes some of the failures in test_apicbase() and test_enable_x2apic(). There are still a significant number of test failures remaining, but these equally apply to TCG and aren't specific to these hvf patches. (Of course it would be nice to fix them, but I've been spending a lot of time and energy on the PV Graphics and vmapple patch set… hopefully that will eventually be merged and I might get a chance to get some of the other small improvements I've got lying around into an upstreamable state.) All the best, Phil > Paolo, please apply this if you have no objections. > > Regards, > Roman >