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

[Julien Grall]

On 01/03/17 19:42, Shanker Donthineni wrote:

Hi Julien,


Hi Shanker,


On 02/28/2017 12:29 PM, Julien Grall wrote:

On 27/02/17 17:20, Andre Przywara wrote:

Direct VLPI injection feature is included in GICv4 architecture. A new
set of VLPI commands are introduced to map ITS vpend/vprop tables, ITTE
setup, and maintenance operations for VLPIs. In case of direct VLPI
injection, domU/dom0 LPI commands are mapped to VLPI commands. Some of
these commands must be applied to a real ITS hardware whenever XEN
receives the ITS commands during runtime.


Any thought on this, how we are going to support a direct VLPI injection
without prolongating dom0/domU ITS commands to hardware at runtime?


direct vLPI injection will indeed require to propagate commands. But as 
the host command queue is shared among multiple guest, we have to 
prevent a guest to overflow the host command queue and affecting other 
guests.


During the discussion for GICv3 ITS support in Xen, we looked at various 
solution (see the various design doc sent by Ian Campbell [1]) and the 
only suitable one for it was to decouple vITS and ITS. This is what 
Andre has implemented in this series.


I don't know yet how we can make things secure for direct vLPI 
injection. For the time being, I think we should focus to get GICv3 ITS 
supported as it is a requirement to get MSI supported.


Once this is done, we can think about integrating directly vLPI in the 
code. Feel free to start a new thread about this.


Cheers,

[1] https://xenbits.xen.org/people/ianc/vits/

--
Julien Grall

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

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

[Shanker Donthineni]

Hi Julien,


On 02/28/2017 12:29 PM, Julien Grall wrote:



On 27/02/17 17:20, Andre Przywara wrote:

Hi,


Hi Andre,


On 24/02/17 19:57, Shanker Donthineni wrote:

Hi Julien,


On 01/31/2017 10:18 AM, Julien Grall wrote:



On 31/01/17 16:02, Jaggi, Manish wrote:



On 1/31/2017 8:47 PM, Julien Grall wrote:



On 31/01/17 14:08, Jaggi, Manish wrote:

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:

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.


This is not the first time I am saying this, just saying "we should
add that another way..." is not helpful. You should also provide
some details on what you would do.


Julien, As you suggested we need to find another way, I assumed you
had something in mind.


I gave suggestions on my e-mail but you may have missed it...


Since we both agree that sbdf!=deviceID, the current series of ITS
patches will program the incorrect deviceID so there is a need to
have a way to map sbdf with deviceID in xen.

One option could be to add a new hypercall to supply sbdf and
deviceID to xen.


... as well as the part where I am saying that I am not in favor to
implement an hypercall temporarily, and against adding a new hypercall
for only a couple of weeks. As you may know PHYSDEV hypercall are part
of the stable ABI and once they are added they cannot be removed.

So we need to be sure the hypercall is necessary. In this case, the
hypercall is not necessary as all the information can be found in the
firmware tables. However this is not implemented yet and part of the
discussion on PCI Passthrough (see [1]).

We need a temporary solution that does not involve any commitment 
on the

ABI until Xen is able to discover PCI.



Why can't  we handle ITS device creation whenever a virtual ITS driver
receives the MAPD command from dom0/domU. In case of dom0, it's 
straight

forward dom0 always passes the real ITS device through MAPD command.
This way we can support PCIe devices without hard-coded MSI(x) limit 
32,

and platform devices transparently. I used the below code to platform
and PCIe device MSI(x) functionality on QDF2400 server platform.


But this breaks our assumption that no ITS commands can ever be
propagated at guest's runtime, which is the cornerstone of this series.
I agree that this is unfortunate and allowing it would simplify things,
but after long discussions we came to the conclusion that it's not
feasible to do so:
A malicious guest could flood the virtual ITS with MAPD commands. Xen
would need to propagate those to the hardware, which relies on the host
command queue to have free slots, which we can't guarantee. For
technical reasons we can't reschedule the guest (because this is an MMIO
trap), also the domain actually triggering the "final" MAPD might not be
the culprit, but an actual legitimate user.
So we agreed upon issuing all hardware ITS commands before a guest
actually starts (DomUs), respectively on hypercalls for Dom0.
I think we can do exceptions for Dom0, since it's not supposed to be
malicious.


Thank you for summarizing the problem :).



Direct VLPI injection feature is included in GICv4 architecture. A new 
set of VLPI commands are introduced to map ITS vpend/vprop tables, ITTE 
setup, and maintenance operations for VLPIs. In case of direct VLPI 
injection, domU/dom0 LPI commands are mapped to VLPI commands. Some of 
these commands must be applied to a real ITS hardware whenever XEN 
receives the ITS commands during runtime.



Any thought on this, how we are going to support a direct VLPI injection 
without prolongating dom0/domU ITS commands to hardware at runtime?


--

Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.


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

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

[Julien Grall]

On 27/02/17 17:20, Andre Przywara wrote:

Hi,


Hi Andre,


On 24/02/17 19:57, Shanker Donthineni wrote:

Hi Julien,


On 01/31/2017 10:18 AM, Julien Grall wrote:



On 31/01/17 16:02, Jaggi, Manish wrote:



On 1/31/2017 8:47 PM, Julien Grall wrote:



On 31/01/17 14:08, Jaggi, Manish wrote:

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:

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.


This is not the first time I am saying this, just saying "we should
add that another way..." is not helpful. You should also provide
some details on what you would do.


Julien, As you suggested we need to find another way, I assumed you
had something in mind.


I gave suggestions on my e-mail but you may have missed it...


Since we both agree that sbdf!=deviceID, the current series of ITS
patches will program the incorrect deviceID so there is a need to
have a way to map sbdf with deviceID in xen.

One option could be to add a new hypercall to supply sbdf and
deviceID to xen.


... as well as the part where I am saying that I am not in favor to
implement an hypercall temporarily, and against adding a new hypercall
for only a couple of weeks. As you may know PHYSDEV hypercall are part
of the stable ABI and once they are added they cannot be removed.

So we need to be sure the hypercall is necessary. In this case, the
hypercall is not necessary as all the information can be found in the
firmware tables. However this is not implemented yet and part of the
discussion on PCI Passthrough (see [1]).

We need a temporary solution that does not involve any commitment on the
ABI until Xen is able to discover PCI.



Why can't  we handle ITS device creation whenever a virtual ITS driver
receives the MAPD command from dom0/domU. In case of dom0, it's straight
forward dom0 always passes the real ITS device through MAPD command.
This way we can support PCIe devices without hard-coded MSI(x) limit 32,
and platform devices transparently. I used the below code to platform
and PCIe device MSI(x) functionality on QDF2400 server platform.


But this breaks our assumption that no ITS commands can ever be
propagated at guest's runtime, which is the cornerstone of this series.
I agree that this is unfortunate and allowing it would simplify things,
but after long discussions we came to the conclusion that it's not
feasible to do so:
A malicious guest could flood the virtual ITS with MAPD commands. Xen
would need to propagate those to the hardware, which relies on the host
command queue to have free slots, which we can't guarantee. For
technical reasons we can't reschedule the guest (because this is an MMIO
trap), also the domain actually triggering the "final" MAPD might not be
the culprit, but an actual legitimate user.
So we agreed upon issuing all hardware ITS commands before a guest
actually starts (DomUs), respectively on hypercalls for Dom0.
I think we can do exceptions for Dom0, since it's not supposed to be
malicious.


Thank you for summarizing the problem :).


So I'd suggest the following:
- To make Dom0 run in this version of the patches, especially with
platform devices, we allow MAPDs to propagate from Dom0.
  - We check whether this device has already  been mapped. If yes, we
map the virtual side and return.
  - If not mapped already, we possibly somehow sanitize the device ID
(using some platform-specific function, for instance) and issue the MAPD
and all the possible MAPTIs to the hardware ITS. We might avoid this in
the future, when we have proper passthrough support in place.


I am not sure why you would need per-platform code to sanitize the 
Device ID. I think a first approach is to trust all input from dom0, we 
can refine this later one by either reading the configuration space for 
PCI, for platform device we would need to come up for possibly a new 
hypercall (this could be discussed in a separate thread).




So PCI devices would be mapped by the PHYSOPS hypercall as before, but
platform devices would be handled via this way.


I don't understand why you still want to implement physdevop hypercalls 
knowing that they will likely get ditched for ARM and don't provide all 
the information we need. It is not possible to know the DeviceID from 
RID without parsing DT and we don't have the number of MSI supported in 
hand.


So it makes no sense to implement those hypercalls.



Does this make sense?


Looking at the implementation of gicv3_its_map_guest_device, for each 
virtual MAPD issued, you will issue one host MAPD command, one host 
MAPTI and INV per event.


This will potentially fill up the host command queue and takes time to 
executed (imagine a SYNC at the end).


So what will you do if the queue is full? 

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

[Andre Przywara]

Hi,

On 24/02/17 19:57, Shanker Donthineni wrote:
> Hi Julien,
> 
> 
> On 01/31/2017 10:18 AM, Julien Grall wrote:
>>
>>
>> On 31/01/17 16:02, Jaggi, Manish wrote:
>>>
>>>
>>> On 1/31/2017 8:47 PM, Julien Grall wrote:


 On 31/01/17 14:08, Jaggi, Manish wrote:
> 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:
> 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.

 This is not the first time I am saying this, just saying "we should
 add that another way..." is not helpful. You should also provide
 some details on what you would do.

>>> Julien, As you suggested we need to find another way, I assumed you
>>> had something in mind.
>>
>> I gave suggestions on my e-mail but you may have missed it...
>>
>>> Since we both agree that sbdf!=deviceID, the current series of ITS
>>> patches will program the incorrect deviceID so there is a need to
>>> have a way to map sbdf with deviceID in xen.
>>>
>>> One option could be to add a new hypercall to supply sbdf and
>>> deviceID to xen.
>>
>> ... as well as the part where I am saying that I am not in favor to
>> implement an hypercall temporarily, and against adding a new hypercall
>> for only a couple of weeks. As you may know PHYSDEV hypercall are part
>> of the stable ABI and once they are added they cannot be removed.
>>
>> So we need to be sure the hypercall is necessary. In this case, the
>> hypercall is not necessary as all the information can be found in the
>> firmware tables. However this is not implemented yet and part of the
>> discussion on PCI Passthrough (see [1]).
>>
>> We need a temporary solution that does not involve any commitment on the
>> ABI until Xen is able to discover PCI.
>>
> 
> Why can't  we handle ITS device creation whenever a virtual ITS driver
> receives the MAPD command from dom0/domU. In case of dom0, it's straight
> forward dom0 always passes the real ITS device through MAPD command.
> This way we can support PCIe devices without hard-coded MSI(x) limit 32,
> and platform devices transparently. I used the below code to platform
> and PCIe device MSI(x) functionality on QDF2400 server platform.

But this breaks our assumption that no ITS commands can ever be
propagated at guest's runtime, which is the cornerstone of this series.
I agree that this is unfortunate and allowing it would simplify things,
but after long discussions we came to the conclusion that it's not
feasible to do so:
A malicious guest could flood the virtual ITS with MAPD commands. Xen
would need to propagate those to the hardware, which relies on the host
command queue to have free slots, which we can't guarantee. For
technical reasons we can't reschedule the guest (because this is an MMIO
trap), also the domain actually triggering the "final" MAPD might not be
the culprit, but an actual legitimate user.
So we agreed upon issuing all hardware ITS commands before a guest
actually starts (DomUs), respectively on hypercalls for Dom0.
I think we can do exceptions for Dom0, since it's not supposed to be
malicious.
So I'd suggest the following:
- To make Dom0 run in this version of the patches, especially with
platform devices, we allow MAPDs to propagate from Dom0.
  - We check whether this device has already  been mapped. If yes, we
map the virtual side and return.
  - If not mapped already, we possibly somehow sanitize the device ID
(using some platform-specific function, for instance) and issue the MAPD
and all the possible MAPTIs to the hardware ITS. We might avoid this in
the future, when we have proper passthrough support in place.

So PCI devices would be mapped by the PHYSOPS hypercall as before, but
platform devices would be handled via this way.

Does this make sense?

I need to work out the details, keep you posted ...

Cheers,
Andre.

> 
> @@ -383,10 +384,17 @@ static int its_handle_mapd(struct virt_its *its,
> uint64_t *cmdptr)
>  int size = its_cmd_get_size(cmdptr);
>  bool valid = its_cmd_get_validbit(cmdptr);
>  paddr_t itt_addr = its_cmd_mask_field(cmdptr, 2, 0, 52) &
> GENMASK(51, 8);
> +int ret;
> 
>  if ( !its->dev_table )
>  return -1;
> 
> +size = size < 4 ? 4 : size;
> +ret = gicv3_its_map_guest_device(hardware_domain, devid, devid,
> size + 1,
> + valid);
> +if (ret < 0)
> +return ret;
> +
> 
> 

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

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

[Julien Grall]

On 24/02/2017 19:57, Shanker Donthineni wrote:

Hi Julien,


Hi Shanker,



On 01/31/2017 10:18 AM, Julien Grall wrote:



On 31/01/17 16:02, Jaggi, Manish wrote:



On 1/31/2017 8:47 PM, Julien Grall wrote:



On 31/01/17 14:08, Jaggi, Manish wrote:

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:

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.


This is not the first time I am saying this, just saying "we should
add that another way..." is not helpful. You should also provide
some details on what you would do.


Julien, As you suggested we need to find another way, I assumed you
had something in mind.


I gave suggestions on my e-mail but you may have missed it...


Since we both agree that sbdf!=deviceID, the current series of ITS
patches will program the incorrect deviceID so there is a need to
have a way to map sbdf with deviceID in xen.

One option could be to add a new hypercall to supply sbdf and
deviceID to xen.


... as well as the part where I am saying that I am not in favor to
implement an hypercall temporarily, and against adding a new hypercall
for only a couple of weeks. As you may know PHYSDEV hypercall are part
of the stable ABI and once they are added they cannot be removed.

So we need to be sure the hypercall is necessary. In this case, the
hypercall is not necessary as all the information can be found in the
firmware tables. However this is not implemented yet and part of the
discussion on PCI Passthrough (see [1]).

We need a temporary solution that does not involve any commitment on the
ABI until Xen is able to discover PCI.



Why can't  we handle ITS device creation whenever a virtual ITS driver
receives the MAPD command from dom0/domU. In case of dom0, it's straight
forward dom0 always passes the real ITS device through MAPD command.


I guess this can work. Note that, on a separate thread (see [1]), I 
suggested to decouple the virtual DeviceID to the physical one for DOM0 
to simply the generation of the IORT.


So we would have to be a bit more clever here. But that's probably a 
separate subject and can go in Xen in separate series.



This way we can support PCIe devices without hard-coded MSI(x) limit 32,
and platform devices transparently. I used the below code to platform
and PCIe device MSI(x) functionality on QDF2400 server platform.

@@ -383,10 +384,17 @@ static int its_handle_mapd(struct virt_its *its,
uint64_t *cmdptr)
 int size = its_cmd_get_size(cmdptr);
 bool valid = its_cmd_get_validbit(cmdptr);
 paddr_t itt_addr = its_cmd_mask_field(cmdptr, 2, 0, 52) &
GENMASK(51, 8);
+int ret;

 if ( !its->dev_table )
 return -1;

+size = size < 4 ? 4 : size;
+ret = gicv3_its_map_guest_device(hardware_domain, devid, devid,
size + 1,
+ valid);
+if (ret < 0)
+return ret;
+



Cheers,

[1] https://lists.xen.org/archives/html/xen-devel/2017-02/msg02782.html

--
Julien Grall

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

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

[Shanker Donthineni]

Hi Julien,


On 01/31/2017 10:18 AM, Julien Grall wrote:



On 31/01/17 16:02, Jaggi, Manish wrote:



On 1/31/2017 8:47 PM, Julien Grall wrote:



On 31/01/17 14:08, Jaggi, Manish wrote:

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:
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.


This is not the first time I am saying this, just saying "we should 
add that another way..." is not helpful. You should also provide 
some details on what you would do.


Julien, As you suggested we need to find another way, I assumed you 
had something in mind.


I gave suggestions on my e-mail but you may have missed it...

Since we both agree that sbdf!=deviceID, the current series of ITS 
patches will program the incorrect deviceID so there is a need to

have a way to map sbdf with deviceID in xen.

One option could be to add a new hypercall to supply sbdf and 
deviceID to xen.


... as well as the part where I am saying that I am not in favor to
implement an hypercall temporarily, and against adding a new hypercall
for only a couple of weeks. As you may know PHYSDEV hypercall are part
of the stable ABI and once they are added they cannot be removed.

So we need to be sure the hypercall is necessary. In this case, the
hypercall is not necessary as all the information can be found in the
firmware tables. However this is not implemented yet and part of the
discussion on PCI Passthrough (see [1]).

We need a temporary solution that does not involve any commitment on the
ABI until Xen is able to discover PCI.



Why can't  we handle ITS device creation whenever a virtual ITS driver 
receives the MAPD command from dom0/domU. In case of dom0, it's straight 
forward dom0 always passes the real ITS device through MAPD command. 
This way we can support PCIe devices without hard-coded MSI(x) limit 32, 
and platform devices transparently. I used the below code to platform 
and PCIe device MSI(x) functionality on QDF2400 server platform.


@@ -383,10 +384,17 @@ static int its_handle_mapd(struct virt_its *its, 
uint64_t *cmdptr)

 int size = its_cmd_get_size(cmdptr);
 bool valid = its_cmd_get_validbit(cmdptr);
 paddr_t itt_addr = its_cmd_mask_field(cmdptr, 2, 0, 52) & 
GENMASK(51, 8);

+int ret;

 if ( !its->dev_table )
 return -1;

+size = size < 4 ? 4 : size;
+ret = gicv3_its_map_guest_device(hardware_domain, devid, devid, 
size + 1,

+ valid);
+if (ret < 0)
+return ret;
+


--
Shanker Donthineni
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.


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

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

[Stefano Stabellini]

On Mon, 30 Jan 2017, Andre Przywara wrote:
> To get MSIs from devices forwarded to a CPU, we need to name the device
> and its MSIs by mapping them to an ITS.
> Since this involves queueing commands to the ITS command queue, we can't
> really afford to do this during the guest's runtime, as this would open
> up a denial-of-service attack vector.
> So we require every device with MSI interrupts to be mapped explicitly by
> Dom0. For Dom0 itself we can just use the existing PCI physdev_op
> hypercalls, which the existing Linux kernel issues already.
> So upon receipt of this hypercall we map the device to the hardware ITS
> and prepare it to be later mapped by the virtual ITS by using the very
> same device ID (for Dom0 only).
> Also we ask for mapping 32 LPIs to cover 32 MSIs that the device may
> use.
> 
> Signed-off-by: Andre Przywara 
> ---
>  xen/arch/arm/physdev.c | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
> index 27bbbda..6e02de4 100644
> --- a/xen/arch/arm/physdev.c
> +++ b/xen/arch/arm/physdev.c
> @@ -9,11 +9,32 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  
>  int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
> +struct physdev_manage_pci manage;
> +u32 devid;
> +int ret;
> +
> +switch (cmd)
> +{
> +case PHYSDEVOP_manage_pci_add:
> +case PHYSDEVOP_manage_pci_remove:
> +if ( copy_from_guest(, arg, 1) != 0 )
> +return -EFAULT;

You need to check that current is the hardware domain first.


> +devid = manage.bus << 8 | manage.devfn;
> +/* Allocate an ITS device table with space for 32 MSIs */

Please explain why 32


> +ret = gicv3_its_map_guest_device(hardware_domain, devid, devid, 
> 5,
> + cmd == 
> PHYSDEVOP_manage_pci_add);
> +
> +return ret;
> +}
> +
>  gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
>  return -ENOSYS;
>  }
> -- 
> 2.9.0
> 

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

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

[Julien Grall]

On 31/01/17 16:02, Jaggi, Manish wrote:



On 1/31/2017 8:47 PM, Julien Grall wrote:



On 31/01/17 14:08, Jaggi, Manish wrote:

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:

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.


This is not the first time I am saying this, just saying "we should add that another 
way..." is not helpful. You should also provide some details on what you would do.


Julien, As you suggested we need to find another way, I assumed you had 
something in mind.


I gave suggestions on my e-mail but you may have missed it...


Since we both agree that sbdf!=deviceID, the current series of ITS patches will 
program the incorrect deviceID so there is a need to
have a way to map sbdf with deviceID in xen.

One option could be to add a new hypercall to supply sbdf and deviceID to xen.


... as well as the part where I am saying that I am not in favor to 
implement an hypercall temporarily, and against adding a new hypercall 
for only a couple of weeks. As you may know PHYSDEV hypercall are part 
of the stable ABI and once they are added they cannot be removed.


So we need to be sure the hypercall is necessary. In this case, the 
hypercall is not necessary as all the information can be found in the 
firmware tables. However this is not implemented yet and part of the 
discussion on PCI Passthrough (see [1]).


We need a temporary solution that does not involve any commitment on the 
ABI until Xen is able to discover PCI.


Regards,

[1] <5cf9128e-e845-2a89-f7c7-ac8616941...@linaro.org>







--
Julien Grall

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

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

[Jaggi, Manish]

On 1/31/2017 8:47 PM, Julien Grall wrote:
>
>
> On 31/01/17 14:08, Jaggi, Manish wrote:
>> 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  on behalf of Andre
>>> Przywara 
>>> Sent: Tuesday, January 31, 2017 12:01 AM
>>> To: Stefano Stabellini; Julien Grall
>>> Cc: xen-de...@lists.xenproject.org; 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(, 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 think there is a miscommunication problem here. By "my e-mail" I was 
> referring to the e-mail on this thread 
> (4a8e35dc-57e5-e493-9a9a-4a91bb8e1...@arm.com). On your e-mail you implied I 
> was not aware of sbdf != DeviceID (see "Do you think sbdf and device ID are 
> same").
>
>
>>> 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. 
>
> This is not the first time I am saying this, just saying "we should add that 
> another way..." is not helpful. You should also provide some details on what 
> you would do.
>
Julien, As you suggested we need to find another way, I assumed you had 
something in mind.
Since we both agree that sbdf!=deviceID, the current series of ITS patches will 
program the incorrect deviceID so there is a need to
have a way to map sbdf with deviceID in xen.

One option could be to add a new hypercall to supply sbdf and deviceID to xen.

 #define PHYSDEVOP_pci_dev_map_msi_specifier33
 struct physdev_pci_dev_map_msi_specifier {
/* IN */
uint16_t seg;
uint8_t bus;
uint8_t devfn;
uint32_t msi_specifier; //DeviceID
 };

(https://lists.xen.org/archives/html/xen-devel/2016-12/msg00224.html)

> For now, you gave no feedbacks on my suggestions and I have no clue what you 
> mean by "agreement hypercall".
>
> Cheers,
>


   
___
Xen-devel mailing list

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

[Julien Grall]

On 31/01/17 14:08, Jaggi, Manish wrote:

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  on behalf of Andre
Przywara 
Sent: Tuesday, January 31, 2017 12:01 AM
To: Stefano Stabellini; Julien Grall
Cc: xen-de...@lists.xenproject.org; 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(, 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 think there is a miscommunication problem here. By "my e-mail" I was 
referring to the e-mail on this thread 
(4a8e35dc-57e5-e493-9a9a-4a91bb8e1...@arm.com). On your e-mail you 
implied I was not aware of sbdf != DeviceID (see "Do you think sbdf and 
device ID are same").




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.


This is not the first time I am saying this, just saying "we should add 
that another way..." is not helpful. You should also provide some 
details on what you would do.


For now, you gave no feedbacks on my suggestions and I have no clue what 
you mean by "agreement hypercall".


Cheers,

--
Julien Grall

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

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

[Jaggi, Manish]

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  on behalf of Andre
> Przywara 
> Sent: Tuesday, January 31, 2017 12:01 AM
> To: Stefano Stabellini; Julien Grall
> Cc: xen-de...@lists.xenproject.org; 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(, 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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Julien Grall]

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  on behalf of Andre
Przywara 
Sent: Tuesday, January 31, 2017 12:01 AM
To: Stefano Stabellini; Julien Grall
Cc: xen-de...@lists.xenproject.org; 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(, 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@lists.xen.org
https://lists.xen.org/xen-devel

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

[Jaggi, Manish]

Hi Julien,


On 1/31/2017 6:13 PM, Julien Grall wrote:
>
>
> On 31/01/17 10:29, Jaggi, Manish wrote:
>>>
>>> From: Xen-devel  on behalf of Andre 
>>> Przywara 
>>> Sent: Tuesday, January 31, 2017 12:01 AM
>>> To: Stefano Stabellini; Julien Grall
>>> Cc: xen-de...@lists.xenproject.org; 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(, 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 is programmed with 
wrong deviceID.
I suggest this series to include another way as well.
> 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 ?
> However, I am not sure whether we are going to need those hypercalls 
> when Xen will gain support of PCI. There are some discussion to let 
> Xen scanning the PCI devices, and therefore the hypercalls will be used.
>
> Today, the hypercall is called by Linux on ARM, but it might not be 
> the case in the future. If we decide to implement it today, it means 
> that we will not be able to remove it from Linux from compatibility 
> reasons.
>
> So I would be more in favor of having a per-platform list of devices 
> to support for the time being. So we can get GICv3 ITS working with 
> Device Tree until Xen gain support of PCI. Stefano, Andre, any opinions?
> Cheers,
>


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

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

[Jaggi, Manish]

Hi Julien,


On 1/31/2017 6:13 PM, Julien Grall wrote:
>
>
> On 31/01/17 10:29, Jaggi, Manish wrote:
>>>
>>> From: Xen-devel  on behalf of Andre 
>>> Przywara 
>>> Sent: Tuesday, January 31, 2017 12:01 AM
>>> To: Stefano Stabellini; Julien Grall
>>> Cc: xen-de...@lists.xenproject.org; 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(, 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.

> 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 ?
> However, I am not sure whether we are going to need those hypercalls 
> when Xen will gain support of PCI. There are some discussion to let 
> Xen scanning the PCI devices, and therefore the hypercalls will be used.
>
> Today, the hypercall is called by Linux on ARM, but it might not be 
> the case in the future. If we decide to implement it today, it means 
> that we will not be able to remove it from Linux from compatibility 
> reasons.
>
> So I would be more in favor of having a per-platform list of devices 
> to support for the time being. So we can get GICv3 ITS working with 
> Device Tree until Xen gain support of PCI. Stefano, Andre, any opinions?
> Cheers,
>


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

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

[Julien Grall]

On 31/01/17 10:29, Jaggi, Manish wrote:


From: Xen-devel  on behalf of Andre Przywara 

Sent: Tuesday, January 31, 2017 12:01 AM
To: Stefano Stabellini; Julien Grall
Cc: xen-de...@lists.xenproject.org; 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(, 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.


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.


However, I am not sure whether we are going to need those hypercalls 
when Xen will gain support of PCI. There are some discussion to let Xen 
scanning the PCI devices, and therefore the hypercalls will be used.


Today, the hypercall is called by Linux on ARM, but it might not be the 
case in the future. If we decide to implement it today, it means that we 
will not be able to remove it from Linux from compatibility reasons.


So I would be more in favor of having a per-platform list of devices to 
support for the time being. So we can get GICv3 ITS working with Device 
Tree until Xen gain support of PCI. Stefano, Andre, any opinions?


Cheers,

--
Julien Grall

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

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

[Jaggi, Manish]

Hi Andre,
>
>From: Xen-devel  on behalf of Andre Przywara 
>
>Sent: Tuesday, January 31, 2017 12:01 AM
>To: Stefano Stabellini; Julien Grall
>Cc: xen-de...@lists.xenproject.org; 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(, 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. 
As sbdf and deviceID returned from msi_map calls might not be same.

>+
>+    return ret;
>+    }
>+
> gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
> return -ENOSYS;
> }
>-- 
>2.9.0
>

-Manish


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

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

[Andre Przywara]

To get MSIs from devices forwarded to a CPU, we need to name the device
and its MSIs by mapping them to an ITS.
Since this involves queueing commands to the ITS command queue, we can't
really afford to do this during the guest's runtime, as this would open
up a denial-of-service attack vector.
So we require every device with MSI interrupts to be mapped explicitly by
Dom0. For Dom0 itself we can just use the existing PCI physdev_op
hypercalls, which the existing Linux kernel issues already.
So upon receipt of this hypercall we map the device to the hardware ITS
and prepare it to be later mapped by the virtual ITS by using the very
same device ID (for Dom0 only).
Also we ask for mapping 32 LPIs to cover 32 MSIs that the device may
use.

Signed-off-by: Andre Przywara 
---
 xen/arch/arm/physdev.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/xen/arch/arm/physdev.c b/xen/arch/arm/physdev.c
index 27bbbda..6e02de4 100644
--- a/xen/arch/arm/physdev.c
+++ b/xen/arch/arm/physdev.c
@@ -9,11 +9,32 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 
 
 int do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
+struct physdev_manage_pci manage;
+u32 devid;
+int ret;
+
+switch (cmd)
+{
+case PHYSDEVOP_manage_pci_add:
+case PHYSDEVOP_manage_pci_remove:
+if ( copy_from_guest(, 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);
+
+return ret;
+}
+
 gdprintk(XENLOG_DEBUG, "PHYSDEVOP cmd=%d: not implemented\n", cmd);
 return -ENOSYS;
 }
-- 
2.9.0


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