On Mon, Mar 20, 2023 at 04:29:25PM +0100, Jan Beulich wrote:
> On 20.03.2023 16:16, Roger Pau Monné wrote:
> > @@ -244,12 +242,18 @@ static void vioapic_write_redirent(
> > }
> > else
> > {
> > + int ret;
> > +
> > unmasked = ent.fields.mask;
> > /* Remote IRR and Delivery Status are read-only. */
> > ent.bits = ((ent.bits >> 32) << 32) | val;
> > ent.fields.delivery_status = 0;
> > ent.fields.remote_irr = pent->fields.remote_irr;
> > unmasked = unmasked && !ent.fields.mask;
> > + ret = mp_register_gsi(gsi, ent.fields.trig_mode,
> > ent.fields.polarity);
> > + if ( ret && ret != -EEXIST )
> > + gprintk(XENLOG_WARNING, "vioapic: error registering GSI %u:
> > %d\n",
> > + gsi, ret);
> > }
>
> I assume this is only meant to be experimental, as I'm missing confinement
> to Dom0 here.
Indeed. I've attached a fixed version below, let's make sure this
doesn't influence testing.
> I also question this when the mask bit as set, as in that
> case neither the trigger mode bit nor the polarity one can be relied upon.
> At which point it would look to me as if it was necessary for Dom0 to use
> a hypercall instead (which naturally would then be PHYSDEVOP_setup_gsi).
AFAICT Linux does correctly set the trigger/polarity even when the
pins are masked, so this should be safe as a proof of concept. Let's
first figure out whether the issue is really with the lack of setup of
the IO-APIC pins. At the end without input from Ray this is just a
wild guess.
Regards, Roger.
----
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 405d0a95af..cc53a3bd12 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -86,6 +86,8 @@ long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
{
case PHYSDEVOP_map_pirq:
case PHYSDEVOP_unmap_pirq:
+ break;
+
case PHYSDEVOP_eoi:
case PHYSDEVOP_irq_status_query:
case PHYSDEVOP_get_free_pirq:
diff --git a/xen/arch/x86/hvm/vioapic.c b/xen/arch/x86/hvm/vioapic.c
index 41e3c4d5e4..64f7b5bcc5 100644
--- a/xen/arch/x86/hvm/vioapic.c
+++ b/xen/arch/x86/hvm/vioapic.c
@@ -180,9 +180,7 @@ static int vioapic_hwdom_map_gsi(unsigned int gsi, unsigned
int trig,
/* Interrupt has been unmasked, bind it now. */
ret = mp_register_gsi(gsi, trig, pol);
- if ( ret == -EEXIST )
- return 0;
- if ( ret )
+ if ( ret && ret != -EEXIST )
{
gprintk(XENLOG_WARNING, "vioapic: error registering GSI %u: %d\n",
gsi, ret);
@@ -250,6 +248,16 @@ static void vioapic_write_redirent(
ent.fields.delivery_status = 0;
ent.fields.remote_irr = pent->fields.remote_irr;
unmasked = unmasked && !ent.fields.mask;
+ if ( is_hardware_domain(d) )
+ {
+ int ret = mp_register_gsi(gsi, ent.fields.trig_mode,
+ ent.fields.polarity);
+
+ if ( ret && ret != -EEXIST )
+ gprintk(XENLOG_WARNING,
+ "vioapic: error registering GSI %u: %d\n",
+ gsi, ret);
+ }
}
*pent = ent;
