Re: [Xen-devel] [PATCH 09/28] ARM: GICv3 ITS: map device and LPIs to the ITS on physdev_op hypercall

[Julien Grall <julien.grall@xxxxxxx>]

On 31/01/17 13:19, Jaggi, Manish wrote:
> On 1/31/2017 6:13 PM, Julien Grall wrote:
> > On 31/01/17 10:29, Jaggi, Manish wrote:
> > > > From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxx> on behalf of Andre
> > > > Przywara <andre.przywara@xxxxxxx>
> > > > Sent: Tuesday, January 31, 2017 12:01 AM
> > > > To: Stefano Stabellini; Julien Grall
> > > > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Vijay Kilari
> > > > Subject: [Xen-devel] [PATCH 09/28] ARM: GICv3 ITS: map device and
> > > > LPIs to the ITS on physdev_op hypercall
> > > [snip]
> > > >  int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
> > > >  {
> > > > +    struct physdev_manage_pci manage;
> > > > +    u32 devid;
> > > > +    int ret;
> > > > +
> > > > +    switch (cmd)
> > > > +    {
> > > You might alos need to  PHYSDEVOP_pci_device_add hypercall also.
> > >
> > > > +        case PHYSDEVOP_manage_pci_add:
> > > > +        case PHYSDEVOP_manage_pci_remove:
> > > > +            if ( copy_from_guest(&manage, arg, 1) != 0 )
> > > > +                return -EFAULT;
> > > > +
> > > > +            devid = manage.bus << 8 | manage.devfn;
> > > > +            /* Allocate an ITS device table with space for 32 MSIs */
> > > > +            ret = gicv3_its_map_guest_device(hardware_domain,
> > > > devid, devid, 5,
> > > > +                                             cmd ==
> > > > PHYSDEVOP_manage_pci_add);
> > > Based on 4.9 kernel, is the deivce ID plain sBDF or it is
> > > returnedfrom of_msi_map_rid /  iort_msi_map_rid ?
> > > I believe there needs to be set this as requirement on the calle of
> > > hypercall.
> > The requirement of the hypercall is already defined and cannot be
> > changed. So if it does not provide the correct information, then we
> > need to find another way to get the DeviceID.
> Do you think sbdf and device ID are same ? If you recollect your
> comments last year sbdf != DeviceID.
> for this series it has to be passed correctly otherwise ITS would be programmed 
> incorrectly.
> I suggest this series to include another way as well.
Thank you sherlock, if you had read my e-mail entirely you would have 
noticed I never said sbdf == DeviceID and actually provided insight on 
the problem and suggest solutions.
I would recommend you to do the same in the future. It would help to get 
the code much faster in Xen.
> > In case of ACPI, we should be able to get those informations from the
> > IORT as the segment number is defined in the firmware tables. But for
> > Device Tree, we would need DOM0 and Xen to agree on the segment number.
> Is there any agreement hypercall used with this series ?
From xen/include/public/physdev.h

struct physdev_manage_pci {
    /* IN */
    uint8_t bus;
    uint8_t devfn;
};

struct physdev_manage_pci_ext {
    /* IN */
    uint8_t bus;
    uint8_t devfn;
    unsigned is_extfn;
    unsigned is_virtfn;
    struct {
        uint8_t bus;
        uint8_t devfn;
    } physfn;
};

Let me know how you could encode a DeviceID in those hypercalls.

Cheers,

--
Julien Grall

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