Re: [Xen-devel] [PATCH v5 2/3] x86/pt: enable binding of GSIs to a PVH Dom0

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Fri, Jun 23, 2017 at 09:26:10AM +0100, Roger Pau Monne wrote:
> On Thu, Jun 22, 2017 at 03:40:52AM -0600, Jan Beulich wrote:
> > >>> On 20.06.17 at 11:15, <roger.pau@xxxxxxxxxx> wrote:
> > > @@ -472,7 +514,29 @@ int pt_irq_create_bind(
> > >                  pirq_dpci->flags = HVM_IRQ_DPCI_MAPPED |
> > >                                     HVM_IRQ_DPCI_MACH_PCI |
> > >                                     HVM_IRQ_DPCI_GUEST_PCI;
> > > -                share = BIND_PIRQ__WILL_SHARE;
> > > +                if ( !is_hardware_domain(d) )
> > > +                    share = BIND_PIRQ__WILL_SHARE;
> > > +                else
> > > +                {
> > > +                    unsigned int pin;
> > > +                    const struct hvm_vioapic *vioapic = gsi_vioapic(d,
> > > +                                                                    
> > > guest_gsi,
> > > +                                                                    
> > > &pin);
> > > +
> > > +                    if ( !vioapic )
> > > +                    {
> > > +                        ASSERT_UNREACHABLE();
> > > +                        return -EINVAL;
> > > +                    }
> > > +                    pirq_dpci->flags |= HVM_IRQ_DPCI_IDENTITY_GSI;
> > > +                    /*
> > > +                     * Check if the corresponding vIO APIC pin is 
> > > configured
> > > +                     * level or edge trigger, level triggered interrupts 
> > > will
> > > +                     * be marked as shareable.
> > > +                     */
> > > +                    ASSERT(!vioapic->redirtbl[pin].fields.mask);
> > > +                    share = vioapic->redirtbl[pin].fields.trig_mode;
> > > +                }
> > 
> > I'm sorry for paying attention to this only now, but the vIO-APIC
> > code here really should be converted into a
> > vioapic_get_trigger_mode() helper living in vioapic.[ch]. The same
> > I think actually holds for the two uses of gsi_vioapic(), and that
> > function should really have been kept static to vioapic.c.
> 
> We will need a couple more of helper functions then,
> vioapic_irq_positive_edge for example reads the trigger mode, but it
> also checks and sets the IRR and pt_irq_vector gets the vector.
> 
> Maybe a vioapic_get_redirection_entry?

I clearly didn't have enough coffee, forget about the
vioapic_irq_positive_edge faff. pt_irq_vector needs the vector and
pt_irq_masked needs the masking bit.

Would you be fine with me introducing
vioapic_get_{trigger_mode,vector}?

Thanks, Roger.

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