Re: [Xen-devel] [PATCH v3 for-next 3/3] x86/vioapic: bind interrupts to PVH Dom0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.05.17 at 16:48, <roger.pau@xxxxxxxxxx> wrote:
> On Tue, May 30, 2017 at 04:05:39AM -0600, Jan Beulich wrote:
>> >>> On 17.05.17 at 17:15, <roger.pau@xxxxxxxxxx> wrote:
>> > Changes since v2:
>> >  - s/vioapic_dom0_map_gsi/vioapic_hwdom_map_gsi/.
>> >  - Don't set hvm_domid in xen_domctl_bind_pt_irq_t (it's ignored).
>> 
>> The implication of the respective earlier comment was for there to
>> first be a prereq patch added removing this dead field. Otherwise
>> not setting the field is a latent bug.
> 
> I've added a pre-patch to get rid of hvm_domid in the bind struct.
> 
>> > --- a/xen/arch/x86/hvm/vioapic.c
>> > +++ b/xen/arch/x86/hvm/vioapic.c
>> > @@ -158,6 +158,52 @@ static int vioapic_read(
>> >      return X86EMUL_OKAY;
>> >  }
>> >  
>> > +static int vioapic_hwdom_map_gsi(unsigned int gsi, unsigned int trig,
>> > +                                 unsigned int pol)
>> > +{
>> > +    struct domain *d = current->domain;
>> > +    xen_domctl_bind_pt_irq_t pt_irq_bind = {
>> > +        .irq_type = PT_IRQ_TYPE_PCI,
>> > +        .machine_irq = gsi,
>> 
>> Actually you still set the field, just that this is no implicit. Hence the
>> latent bug reduces to just the hwdom != Dom0 case, but anyway.
> 
> What do you mean by implicit? Is that because I'm passing d to the
> bind function?

For one I see that me having misspelled thing ("is now implicit") may
have caused misunderstanding. But what I meant anyway was that
by omitting the initializer you implicitly initialize the field to zero,
which is correct if and only if Dom0 == hwdom. But the discussion is
moot with the field gone.

Jan


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