On 4/2/2024 10:31 PM, Michael S. Tsirkin wrote:
On Tue, Apr 02, 2024 at 09:18:44PM +0800, Xiaoyao Li wrote:
On 4/2/2024 6:02 PM, Michael S. Tsirkin wrote:
On Tue, Apr 02, 2024 at 04:25:16AM -0400, Xiaoyao Li wrote:
Set MADT.FLAGS[bit 0].PCAT_COMPAT based on x86ms->pic.

Signed-off-by: Xiaoyao Li <xiaoyao...@intel.com>

Please include more info in the commit log:
what is the behaviour you observe, why it is wrong,
how does the patch fix it, what is guest behaviour
before and after.

Sorry, I thought it was straightforward.

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 for x86 machine, the PCAT_COMPAT bit needs to be
cleared. Otherwise, the guest thinks there is a present PIC even it is
booted with pic=off on QEMU.

(I haven't seen real issue from Linux guest. The user of PIC inside guest
seems only the pit calibration. Whether pit calibration is triggered depends
on other things. But logically, current code is wrong, we need to fix it
anyway.

@Isaku, please share more info if you have)


+ Kirill,

It seems to have issue with legacy irqs with PCAT_COMPAT set 1 while no PIC on QEMU side. Kirill, could you elaborate it?


That's sufficient, thanks! Pls put this in commit log and resubmit.

The commit log and the subject should not repeat
what the diff already states.

---
   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);
--
2.34.1




Reply via email to