Re: [PATCH v1 2/2] hw/i386: Make pic a property of common x86 base machine type

2022-01-11 Thread Xiaoyao Li 

+ Paolo

On 1/11/2022 3:35 PM, Xiaoyao Li wrote:

Legacy PIC (8259) cannot be supported for TDX guests since TDX module
doesn't allow directly interrupt injection.  Using posted interrupts
for the PIC is not a viable option as the guest BIOS/kernel will not
do EOI for PIC IRQs, i.e. will leave the vIRR bit set.

Make PIC the property of common x86 machine type. Hence all x86
machines, including microvm, can disable it.

Signed-off-by: Xiaoyao Li 
---
  hw/i386/microvm.c | 27 +--
  hw/i386/pc_piix.c |  4 +++-
  hw/i386/pc_q35.c  |  4 +++-
  hw/i386/x86.c | 25 +
  include/hw/i386/microvm.h |  2 --
  include/hw/i386/x86.h |  2 ++
  6 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 89b555a2f584..754f1d0593e5 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -247,7 +247,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
  x86ms->pci_irq_mask = 0;
  }
  
-if (mms->pic == ON_OFF_AUTO_ON || mms->pic == ON_OFF_AUTO_AUTO) {

+if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
  qemu_irq *i8259;
  
  i8259 = i8259_init(isa_bus, x86_allocate_cpu_irq());

@@ -491,23 +491,6 @@ static void microvm_machine_reset(MachineState *machine)
  }
  }
  
-static void microvm_machine_get_pic(Object *obj, Visitor *v, const char *name,

-void *opaque, Error **errp)
-{
-MicrovmMachineState *mms = MICROVM_MACHINE(obj);
-OnOffAuto pic = mms->pic;
-
-visit_type_OnOffAuto(v, name, , errp);
-}
-
-static void microvm_machine_set_pic(Object *obj, Visitor *v, const char *name,
-void *opaque, Error **errp)
-{
-MicrovmMachineState *mms = MICROVM_MACHINE(obj);
-
-visit_type_OnOffAuto(v, name, >pic, errp);
-}
-
  static void microvm_machine_get_rtc(Object *obj, Visitor *v, const char *name,
  void *opaque, Error **errp)
  {
@@ -632,7 +615,6 @@ static void microvm_machine_initfn(Object *obj)
  MicrovmMachineState *mms = MICROVM_MACHINE(obj);
  
  /* Configuration */

-mms->pic = ON_OFF_AUTO_AUTO;
  mms->rtc = ON_OFF_AUTO_AUTO;
  mms->pcie = ON_OFF_AUTO_AUTO;
  mms->ioapic2 = ON_OFF_AUTO_AUTO;
@@ -684,13 +666,6 @@ static void microvm_class_init(ObjectClass *oc, void *data)
  
  x86mc->fwcfg_dma_enabled = true;
  
-object_class_property_add(oc, MICROVM_MACHINE_PIC, "OnOffAuto",

-  microvm_machine_get_pic,
-  microvm_machine_set_pic,
-  NULL, NULL);
-object_class_property_set_description(oc, MICROVM_MACHINE_PIC,
-"Enable i8259 PIC");
-
  object_class_property_add(oc, MICROVM_MACHINE_RTC, "OnOffAuto",
microvm_machine_get_rtc,
microvm_machine_set_rtc,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7c7790a5ce34..d05683cd0d77 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -218,7 +218,9 @@ static void pc_init1(MachineState *machine,
  }
  isa_bus_irqs(isa_bus, x86ms->gsi);
  
-pc_i8259_create(isa_bus, gsi_state->i8259_irq);

+if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
+pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+}
  
  if (pcmc->pci_enabled) {

  ioapic_init_gsi(gsi_state, "i440fx");
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1780f79bc127..58e7e693f9e2 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -265,7 +265,9 @@ static void pc_q35_init(MachineState *machine)
  pci_bus_set_route_irq_fn(host_bus, ich9_route_intx_pin_to_irq);
  isa_bus = ich9_lpc->isa_bus;
  
-pc_i8259_create(isa_bus, gsi_state->i8259_irq);

+if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
+pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+}
  
  if (pcmc->pci_enabled) {

  ioapic_init_gsi(gsi_state, "q35");
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 744a50937761..d4a4c0ec8f61 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1243,6 +1243,23 @@ static void x86_machine_set_pit(Object *obj, Visitor *v, 
const char *name,
  visit_type_OnOffAuto(v, name, >pit, errp);
  }
  
+static void x86_machine_get_pic(Object *obj, Visitor *v, const char *name,

+void *opaque, Error **errp)
+{
+X86MachineState *x86ms = X86_MACHINE(obj);
+OnOffAuto pic = x86ms->pic;
+
+visit_type_OnOffAuto(v, name, , errp);
+}
+
+static void x86_machine_set_pic(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+X86MachineState *x86ms = X86_MACHINE(obj);
+
+visit_type_OnOffAuto(v, name, >pic, errp);
+}
+
  static char *x86_machine_get_oem_id(Object *obj, Error **errp)
  {

Re: [PATCH v1 2/2] hw/i386: Make pic a property of common x86 base machine type

2022-01-11 Thread Sergio Lopez 
On Tue, Jan 11, 2022 at 03:35:28PM +0800, Xiaoyao Li wrote:
> Legacy PIC (8259) cannot be supported for TDX guests since TDX module
> doesn't allow directly interrupt injection.  Using posted interrupts
> for the PIC is not a viable option as the guest BIOS/kernel will not
> do EOI for PIC IRQs, i.e. will leave the vIRR bit set.
> 
> Make PIC the property of common x86 machine type. Hence all x86
> machines, including microvm, can disable it.
> 
> Signed-off-by: Xiaoyao Li 
> ---
>  hw/i386/microvm.c | 27 +--
>  hw/i386/pc_piix.c |  4 +++-
>  hw/i386/pc_q35.c  |  4 +++-
>  hw/i386/x86.c | 25 +
>  include/hw/i386/microvm.h |  2 --
>  include/hw/i386/x86.h |  2 ++
>  6 files changed, 34 insertions(+), 30 deletions(-)

Reviewed-by: Sergio Lopez 


signature.asc
Description: PGP signature

[PATCH v1 2/2] hw/i386: Make pic a property of common x86 base machine type

2022-01-10 Thread Xiaoyao Li 
Legacy PIC (8259) cannot be supported for TDX guests since TDX module
doesn't allow directly interrupt injection.  Using posted interrupts
for the PIC is not a viable option as the guest BIOS/kernel will not
do EOI for PIC IRQs, i.e. will leave the vIRR bit set.

Make PIC the property of common x86 machine type. Hence all x86
machines, including microvm, can disable it.

Signed-off-by: Xiaoyao Li 
---
 hw/i386/microvm.c | 27 +--
 hw/i386/pc_piix.c |  4 +++-
 hw/i386/pc_q35.c  |  4 +++-
 hw/i386/x86.c | 25 +
 include/hw/i386/microvm.h |  2 --
 include/hw/i386/x86.h |  2 ++
 6 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 89b555a2f584..754f1d0593e5 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -247,7 +247,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
 x86ms->pci_irq_mask = 0;
 }
 
-if (mms->pic == ON_OFF_AUTO_ON || mms->pic == ON_OFF_AUTO_AUTO) {
+if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
 qemu_irq *i8259;
 
 i8259 = i8259_init(isa_bus, x86_allocate_cpu_irq());
@@ -491,23 +491,6 @@ static void microvm_machine_reset(MachineState *machine)
 }
 }
 
-static void microvm_machine_get_pic(Object *obj, Visitor *v, const char *name,
-void *opaque, Error **errp)
-{
-MicrovmMachineState *mms = MICROVM_MACHINE(obj);
-OnOffAuto pic = mms->pic;
-
-visit_type_OnOffAuto(v, name, , errp);
-}
-
-static void microvm_machine_set_pic(Object *obj, Visitor *v, const char *name,
-void *opaque, Error **errp)
-{
-MicrovmMachineState *mms = MICROVM_MACHINE(obj);
-
-visit_type_OnOffAuto(v, name, >pic, errp);
-}
-
 static void microvm_machine_get_rtc(Object *obj, Visitor *v, const char *name,
 void *opaque, Error **errp)
 {
@@ -632,7 +615,6 @@ static void microvm_machine_initfn(Object *obj)
 MicrovmMachineState *mms = MICROVM_MACHINE(obj);
 
 /* Configuration */
-mms->pic = ON_OFF_AUTO_AUTO;
 mms->rtc = ON_OFF_AUTO_AUTO;
 mms->pcie = ON_OFF_AUTO_AUTO;
 mms->ioapic2 = ON_OFF_AUTO_AUTO;
@@ -684,13 +666,6 @@ static void microvm_class_init(ObjectClass *oc, void *data)
 
 x86mc->fwcfg_dma_enabled = true;
 
-object_class_property_add(oc, MICROVM_MACHINE_PIC, "OnOffAuto",
-  microvm_machine_get_pic,
-  microvm_machine_set_pic,
-  NULL, NULL);
-object_class_property_set_description(oc, MICROVM_MACHINE_PIC,
-"Enable i8259 PIC");
-
 object_class_property_add(oc, MICROVM_MACHINE_RTC, "OnOffAuto",
   microvm_machine_get_rtc,
   microvm_machine_set_rtc,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7c7790a5ce34..d05683cd0d77 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -218,7 +218,9 @@ static void pc_init1(MachineState *machine,
 }
 isa_bus_irqs(isa_bus, x86ms->gsi);
 
-pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
+pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+}
 
 if (pcmc->pci_enabled) {
 ioapic_init_gsi(gsi_state, "i440fx");
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1780f79bc127..58e7e693f9e2 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -265,7 +265,9 @@ static void pc_q35_init(MachineState *machine)
 pci_bus_set_route_irq_fn(host_bus, ich9_route_intx_pin_to_irq);
 isa_bus = ich9_lpc->isa_bus;
 
-pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+if (x86ms->pic == ON_OFF_AUTO_ON || x86ms->pic == ON_OFF_AUTO_AUTO) {
+pc_i8259_create(isa_bus, gsi_state->i8259_irq);
+}
 
 if (pcmc->pci_enabled) {
 ioapic_init_gsi(gsi_state, "q35");
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 744a50937761..d4a4c0ec8f61 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -1243,6 +1243,23 @@ static void x86_machine_set_pit(Object *obj, Visitor *v, 
const char *name,
 visit_type_OnOffAuto(v, name, >pit, errp);
 }
 
+static void x86_machine_get_pic(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+X86MachineState *x86ms = X86_MACHINE(obj);
+OnOffAuto pic = x86ms->pic;
+
+visit_type_OnOffAuto(v, name, , errp);
+}
+
+static void x86_machine_set_pic(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+X86MachineState *x86ms = X86_MACHINE(obj);
+
+visit_type_OnOffAuto(v, name, >pic, errp);
+}
+
 static char *x86_machine_get_oem_id(Object *obj, Error **errp)
 {
 X86MachineState *x86ms = X86_MACHINE(obj);
@@ -1333,6 +1350,7 @@ static void x86_machine_initfn(Object *obj)