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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.06.17 at 10:38, <roger.pau@xxxxxxxxxx> wrote:
> 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}?

These and vioapic_get_maskbit() (or just vioapic_get_mask()).

Jan


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