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

["Jaggi, Manish" <Manish.Jaggi@xxxxxxxxxx>]

Hi Julien, 

On 1/31/2017 7:16 PM, Julien Grall wrote:
> 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.
>
If you please read 4 lines above I wrote sbdf != DeviceID.
> 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.
>
If you please go back to your comment where you wrote "we need to find another 
way to get the DeviceID", I was referring that we should add that another way 
in this series so that correct DeviceID is programmed in ITS.
> Cheers,
>


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