[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

That may be a middle ground, yet I'd personally prefer to have
separate helpers (maybe Andrew could break ties here). If you
use this more generic one, please make sure you either return
the entry by value, or a const pointer to it (to document that
callers aren't supposed to fiddle with the live value).

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.