Re: [Xen-devel] [PATCH 5/5] x86/vioapic: bind interrupts to PVH Dom0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.04.17 at 15:44, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, Apr 18, 2017 at 06:35:57AM -0600, Jan Beulich wrote:
>> >>> On 27.03.17 at 12:44, <roger.pau@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/hvm/vioapic.c
>> > +++ b/xen/arch/x86/hvm/vioapic.c
>> > @@ -199,6 +199,34 @@ static void vioapic_write_redirent(
>> >          unmasked = unmasked && !ent.fields.mask;
>> >      }
>> >  
>> > +    if ( is_hardware_domain(d) && unmasked )
>> > +    {
>> > +        xen_domctl_bind_pt_irq_t pt_irq_bind = {
>> > +            .irq_type = PT_IRQ_TYPE_GSI,
>> > +            .machine_irq = gsi,
>> > +            .u.gsi.gsi = gsi,
>> > +            .hvm_domid = DOMID_SELF,
>> > +        };
>> > +        int ret, pirq = gsi;
>> > +
>> > +        /* Interrupt has been unmasked, bind it now. */
>> > +        ret = mp_register_gsi(gsi, ent.fields.trig_mode, 
>> > ent.fields.polarity);
>> > +        if ( ret && ret != -EEXIST )
>> > +        {
>> > +            gdprintk(XENLOG_WARNING,
>> > +                     "%s: error registering GSI %u: %d\n", __func__, gsi, 
>> > ret);
>> > +        }
>> > +        if ( !ret )
>> > +        {
>> > +            ret = physdev_map_pirq(DOMID_SELF, MAP_PIRQ_TYPE_GSI, &pirq, 
>> > &pirq,
>> > +                                   NULL);
>> 
>> With this call you basically admit that PVH can't really do without
>> physdev ops, just that you hide it behind IO-APIC RTE writes.
> 
> Yes, I just want to get rid of the physdev hypercalls for PVH Dom0, not the
> physdev ops inside of Xen.
> 
>> Along the lines of the comment on the previous patch I wonder
>> though whether you really need to use this function, i.e.
>> whether you can't instead get away with little more than the
>> call to map_domain_pirq() which that function does.
> 
> Well, I could certainly open-code part of physdev_map_pirq here, AFAICT I just
> need to make sure the GSI is not used, and bind it to Dom0, but that's just
> duplicating the code inside of physdev_map_pirq.
> 
> For MSI/MSI-X I certainly need to use physdev_map_pirq, because the logic in
> that case is more complex, so I will have to end up making physdev_map_pirq
> available to external callers in any case.

Hmm, ugly. I'd really like to keep them local as they are - as their
name says - physdev-op specific. Would it perhaps be possible to
split out GSI and MSI specific helpers from it (which you could
then call from here and from the MSI code you're going to add),
or would that still leave too much code duplication?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel