| Message ID | 20240403145953.3082491-1-xiaoyao.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] hw/i386/acpi: Set PCAT_COMPAT bit only when pic is not disabled | expand |
On Wed, 3 Apr 2024 10:59:53 -0400 Xiaoyao Li <xiaoyao.li@intel.com> wrote: > A value 1 of PCAT_COMPAT (bit 0) of MADT.Flags indicates that the system > also has a PC-AT-compatible dual-8259 setup, i.e., the PIC. > > When PIC is not enabled (pic=off) for x86 machine, the PCAT_COMPAT bit > needs to be cleared. Otherwise, the guest thinks there is a present PIC. Can you add to commit message reproducer (aka qemu CLI and relevant logs/symptoms observed on guest side)? > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > changes in v2: > - Clarify more in commit message; > --- > hw/i386/acpi-common.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c > index 20f19269da40..0cc2919bb851 100644 > --- a/hw/i386/acpi-common.c > +++ b/hw/i386/acpi-common.c > @@ -107,7 +107,9 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, > acpi_table_begin(&table, table_data); > /* Local APIC Address */ > build_append_int_noprefix(table_data, APIC_DEFAULT_ADDRESS, 4); > - build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */ > + /* Flags. bit 0: PCAT_COMPAT */ > + build_append_int_noprefix(table_data, > + x86ms->pic != ON_OFF_AUTO_OFF ? 1 : 0 , 4); > > for (i = 0; i < apic_ids->len; i++) { > pc_madt_cpu_entry(i, apic_ids, table_data, false);
On 4/3/2024 11:12 PM, Igor Mammedov wrote: > On Wed, 3 Apr 2024 10:59:53 -0400 > Xiaoyao Li <xiaoyao.li@intel.com> wrote: > >> A value 1 of PCAT_COMPAT (bit 0) of MADT.Flags indicates that the system >> also has a PC-AT-compatible dual-8259 setup, i.e., the PIC. >> >> When PIC is not enabled (pic=off) for x86 machine, the PCAT_COMPAT bit >> needs to be cleared. Otherwise, the guest thinks there is a present PIC. > > Can you add to commit message reproducer (aka qemu CLI and relevant > logs/symptoms observed on guest side)? When booting a VM with "-machine xx,pic=off", there is supposed to be no PIC for the guest. When guest probes PIC, it should find nothing and log of below should be printed: [ 0.155970] Using NULL legacy PIC However, the fact is that no such log printed in guest kernel, with the VM created with "pic=off". This is because guest think there is a present due to pcat_compat is reporte as 1 in MADT. See Linux code of probe_8259A() in arch/x86/kernel/i8259.c > >> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> >> --- >> changes in v2: >> - Clarify more in commit message; >> --- >> hw/i386/acpi-common.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c >> index 20f19269da40..0cc2919bb851 100644 >> --- a/hw/i386/acpi-common.c >> +++ b/hw/i386/acpi-common.c >> @@ -107,7 +107,9 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, >> acpi_table_begin(&table, table_data); >> /* Local APIC Address */ >> build_append_int_noprefix(table_data, APIC_DEFAULT_ADDRESS, 4); >> - build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */ >> + /* Flags. bit 0: PCAT_COMPAT */ >> + build_append_int_noprefix(table_data, >> + x86ms->pic != ON_OFF_AUTO_OFF ? 1 : 0 , 4); >> >> for (i = 0; i < apic_ids->len; i++) { >> pc_madt_cpu_entry(i, apic_ids, table_data, false); >
Queued, thanks. Paolo
On Mon, Apr 15, 2024 at 11:44:50AM +0200, Paolo Bonzini wrote: > Queued, thanks. > > Paolo I actually had it queued too - it's not 9.0 material though. If you queue it don't forget to tweak the commit log manually.
On Mon, Apr 15, 2024 at 05:51:56AM -0400, Michael S. Tsirkin wrote: > On Mon, Apr 15, 2024 at 11:44:50AM +0200, Paolo Bonzini wrote: > > Queued, thanks. > > > > Paolo > > I actually had it queued too - it's not 9.0 material though. > If you queue it don't forget to tweak the commit log manually. oh and then you can add my Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > -- > MST
diff --git a/hw/i386/acpi-common.c b/hw/i386/acpi-common.c index 20f19269da40..0cc2919bb851 100644 --- a/hw/i386/acpi-common.c +++ b/hw/i386/acpi-common.c @@ -107,7 +107,9 @@ void acpi_build_madt(GArray *table_data, BIOSLinker *linker, acpi_table_begin(&table, table_data); /* Local APIC Address */ build_append_int_noprefix(table_data, APIC_DEFAULT_ADDRESS, 4); - build_append_int_noprefix(table_data, 1 /* PCAT_COMPAT */, 4); /* Flags */ + /* Flags. bit 0: PCAT_COMPAT */ + build_append_int_noprefix(table_data, + x86ms->pic != ON_OFF_AUTO_OFF ? 1 : 0 , 4); for (i = 0; i < apic_ids->len; i++) { pc_madt_cpu_entry(i, apic_ids, table_data, false);
A value 1 of PCAT_COMPAT (bit 0) of MADT.Flags indicates that the system also has a PC-AT-compatible dual-8259 setup, i.e., the PIC. When PIC is not enabled (pic=off) for x86 machine, the PCAT_COMPAT bit needs to be cleared. Otherwise, the guest thinks there is a present PIC. Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> --- changes in v2: - Clarify more in commit message; --- hw/i386/acpi-common.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)