On 10/4/23 13:51, Michael S. Tsirkin wrote:
On Tue, Sep 26, 2023 at 11:23:53PM +0700, Bui Quang Minh wrote:
On 9/26/23 23:06, Bui Quang Minh wrote:

Version 8 changes,
- Patch 2, 4:
    + Rebase to master and resolve conflicts in these 2 patches

The conflicts when rebasing is due to the commit 9926cf34de5fa15da
("target/i386: Allow elision of kvm_enable_x2apic()"). AFAIK, this commit
adds kvm_enabled() before kvm_enable_x2apic() in the and (&&) expression so
that when kvm_enabled() is known to be false at the compile time
(CONFIG_KVM_IS_POSSIBLE is undefined), the compiler can omit the
kvm_enable_x2apic() in the and expression.

In patch 2, I simply combine the change logic in patch 2 with logic in the
commit 9926cf34de5fa15da.

In patch 4, the end result of version 8 is the same as version 7. I don't
think we need to add the kvm_enabled() to make the expression become

        if (kvm_enabled() && kvm_irqchip_is_split() && !kvm_enable_x2apic())

Because when CONFIG_KVM_IS_POSSIBLE is undefined, kvm_irqchip_is_split() is
known to be false at the compile time too so just keep the expression as

        if (kvm_irqchip_is_split() && !kvm_enable_x2apic())

is enough.

git range-diff feat/tcg-x2apic-v7~5..feat/tcg-x2apic-v7
feat/tcg-x2apic-v8~5..feat/tcg-x2apic-v8

1:  c1d197a230 = 1:  f6e3918e0f i386/tcg: implement x2APIC registers MSR
access
2:  dd96cb0238 ! 2:  54d44a15b6 apic: add support for x2APIC mode
     @@ Commit message

       ## hw/i386/x86.c ##
      @@ hw/i386/x86.c: void x86_cpus_init(X86MachineState *x86ms, int
default_cpu_version)
     -      * Can we support APIC ID 255 or higher?
     -      *
     -      * Under Xen: yes.
     --     * With userspace emulated lapic: no
     -+     * With userspace emulated lapic: checked later in
apic_common_set_id.
     -      * With KVM's in-kernel lapic: only if X2APIC API is enabled.
     +      * both in-kernel lapic and X2APIC userspace API.
            */
     -     if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
     +     if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
      -        (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
      +        kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
               error_report("current -smp configuration requires kernel "
3:  31a5c555a6 = 3:  eb080d1e2c apic, i386/tcg: add x2apic transitions
4:  d78b5c43b4 ! 4:  59f028f119 intel_iommu: allow Extended Interrupt Mode
when using userspace APIC
     @@ hw/i386/intel_iommu.c: static bool vtd_decide_config(IntelIOMMUState
*s, Error *
      -            error_setg(errp, "eim=on requires
accel=kvm,kernel-irqchip=split");
      -            return false;
      -        }
     --        if (!kvm_enable_x2apic()) {
     +-        if (kvm_enabled() && !kvm_enable_x2apic()) {
      +        if (kvm_irqchip_is_split() && !kvm_enable_x2apic()) {
                   error_setg(errp, "eim=on requires support on the KVM side"
                                    "(X2APIC_API, first shipped in v4.7)");
5:  51f558035d = 5:  bc95c3cb60 amd_iommu: report x2APIC support to the
operating system

As the change is minor and does not change the main logic, I keep the
Reviewed-by and Acked-by tags.

Thank you,
Quang Minh.



Causes some build failures:

https://gitlab.com/mstredhat/qemu/-/jobs/5216377483
/builds/mstredhat/qemu/build/../hw/intc/apic.c:1023: undefined reference to 
`raise_exception_ra'

raise_exception_ra is tcg specific so the builds are failed as tcg is disabled. I will remove the use of raise_exception_ra, the invalid register read just returns 0, invalid register write has no effect without raising the exception anymore. The APIC state invalid transition does not raise exception either, just don't change the APIC state. As a side effect, we fail some more KVM unit test of invalid APIC state transition, as they expect to catch exception in these cases. I think it's not a big problem. What's your opinion?

Thank you,
Quang Minh.

Reply via email to