Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-09-01 Thread Michael S. Tsirkin 
On Fri, Sep 01, 2017 at 02:32:59PM +, Diana Madalina Craciun wrote:
> On 08/23/2017 11:29 PM, Edgar E. Iglesias wrote:
> > On Tue, Aug 22, 2017 at 10:04:25PM +0300, Michael S. Tsirkin wrote:
> >> On Tue, Aug 22, 2017 at 03:13:57PM +, Diana Madalina Craciun wrote:
> >>> On 08/11/2017 06:50 PM, Edgar E. Iglesias wrote:
>  On Fri, Aug 11, 2017 at 02:35:28PM +, Diana Madalina Craciun wrote:
> > Hi Edgar,
> >
> > On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
> >> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
> >>> Hi Diana,
> >>> On 23/05/2017 13:12, Diana Craciun wrote:
>  Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
>  Currently, for PCI devices, the requester ID was used as device
>  ID in the virt machine. If the system has multiple masters that
> >>> if the system has multiple root complex?
>  use MSIs a unique ID accross the platform is needed.
> >>> across
>  A static scheme is used and each master is allocated a range of IDs
>  with the formula:
>  DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
>  recommended by SBSA).
> 
>  This ID will be configured in the machine creation and if not 
>  configured
>  the PCI requester ID will be used insteead.
> >>> instead
>  Signed-off-by: Diana Craciun 
>  ---
>   hw/arm/virt.c  | 26 ++
>   hw/pci-host/gpex.c |  6 ++
>   hw/pci/msi.c   |  2 +-
>   hw/pci/pci.c   | 25 +
>   include/hw/arm/virt.h  |  1 +
>   include/hw/pci-host/gpex.h |  2 ++
>   include/hw/pci/pci.h   |  8 
>   kvm-all.c  |  4 ++--
>   8 files changed, 71 insertions(+), 3 deletions(-)
> 
>  diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>  index 5f62a03..a969694 100644
>  --- a/hw/arm/virt.c
>  +++ b/hw/arm/virt.c
>  @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams 
>  platform_bus_params;
>   #define RAMLIMIT_GB 255
>   #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
>   
>  +#define STREAM_ID_RANGE_SIZE 0x1
>  +
>   /* Addresses and sizes of our components.
>    * 0..128MB is space for a flash device so we can run bootrom code 
>  such as UEFI.
>    * 128MB..256MB is used for miscellaneous device I/O.
>  @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
>   [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS 
>  -1 */
>   };
>   
>  +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
>  Currently
>  + * for PCI devices the requester ID was used as device ID. But if 
>  the system has
>  + * multiple masters that use MSIs, the requester ID may cause 
>  deviceID clashes.
>  + * So a unique number is  needed accross the system.
>  + * We are using the following formula:
>  + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
>  + * (as recommanded by SBSA). Currently we do not have an SMMU 
>  emulation, but the
>  + * same formula can be used for the generation of the streamID as 
>  well.
>  + * For each master the device ID will be derrived from the 
>  requester ID using
>  + * the abovemntione formula.
>  + */
> >>> I think most of this comment should only be in the commit message. 
> >>> typos
> >>> in derived and above mentioned.
> >>>
> >>> stream id is the terminology for the id space at the input of the 
> >>> smmu.
> >>> device id is the terminology for the id space at the input of the msi
> >>> controller I think.
> >>>
> >>> RID -> deviceID (no IOMMU)
> >>> RID -> streamID -> deviceID (IOMMU)
> >>>
> >>> I would personally get rid of all streamid uses as the smmu is not yet
> >>> supported and stick to the
> >>> Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
> >>>
>  +
>  +static const uint32_t streamidmap[] = {
>  +[VIRT_PCIE] = 0, /* currently only one PCI controller */
>  +};
>  +
>   static const char *valid_cpus[] = {
>   "cortex-a15",
>   "cortex-a53",
>  @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState 
>  *vms, qemu_irq *pic)
>   hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
>   hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
>   hwaddr base = base_mmio;
>  +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
>

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-09-01 Thread Diana Madalina Craciun 
On 08/23/2017 11:29 PM, Edgar E. Iglesias wrote:
> On Tue, Aug 22, 2017 at 10:04:25PM +0300, Michael S. Tsirkin wrote:
>> On Tue, Aug 22, 2017 at 03:13:57PM +, Diana Madalina Craciun wrote:
>>> On 08/11/2017 06:50 PM, Edgar E. Iglesias wrote:
 On Fri, Aug 11, 2017 at 02:35:28PM +, Diana Madalina Craciun wrote:
> Hi Edgar,
>
> On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
>> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
>>> Hi Diana,
>>> On 23/05/2017 13:12, Diana Craciun wrote:
 Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
 Currently, for PCI devices, the requester ID was used as device
 ID in the virt machine. If the system has multiple masters that
>>> if the system has multiple root complex?
 use MSIs a unique ID accross the platform is needed.
>>> across
 A static scheme is used and each master is allocated a range of IDs
 with the formula:
 DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
 recommended by SBSA).

 This ID will be configured in the machine creation and if not 
 configured
 the PCI requester ID will be used insteead.
>>> instead
 Signed-off-by: Diana Craciun 
 ---
  hw/arm/virt.c  | 26 ++
  hw/pci-host/gpex.c |  6 ++
  hw/pci/msi.c   |  2 +-
  hw/pci/pci.c   | 25 +
  include/hw/arm/virt.h  |  1 +
  include/hw/pci-host/gpex.h |  2 ++
  include/hw/pci/pci.h   |  8 
  kvm-all.c  |  4 ++--
  8 files changed, 71 insertions(+), 3 deletions(-)

 diff --git a/hw/arm/virt.c b/hw/arm/virt.c
 index 5f62a03..a969694 100644
 --- a/hw/arm/virt.c
 +++ b/hw/arm/virt.c
 @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams 
 platform_bus_params;
  #define RAMLIMIT_GB 255
  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
  
 +#define STREAM_ID_RANGE_SIZE 0x1
 +
  /* Addresses and sizes of our components.
   * 0..128MB is space for a flash device so we can run bootrom code 
 such as UEFI.
   * 128MB..256MB is used for miscellaneous device I/O.
 @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS 
 -1 */
  };
  
 +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
 Currently
 + * for PCI devices the requester ID was used as device ID. But if the 
 system has
 + * multiple masters that use MSIs, the requester ID may cause 
 deviceID clashes.
 + * So a unique number is  needed accross the system.
 + * We are using the following formula:
 + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
 + * (as recommanded by SBSA). Currently we do not have an SMMU 
 emulation, but the
 + * same formula can be used for the generation of the streamID as 
 well.
 + * For each master the device ID will be derrived from the requester 
 ID using
 + * the abovemntione formula.
 + */
>>> I think most of this comment should only be in the commit message. typos
>>> in derived and above mentioned.
>>>
>>> stream id is the terminology for the id space at the input of the smmu.
>>> device id is the terminology for the id space at the input of the msi
>>> controller I think.
>>>
>>> RID -> deviceID (no IOMMU)
>>> RID -> streamID -> deviceID (IOMMU)
>>>
>>> I would personally get rid of all streamid uses as the smmu is not yet
>>> supported and stick to the
>>> Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
>>>
 +
 +static const uint32_t streamidmap[] = {
 +[VIRT_PCIE] = 0, /* currently only one PCI controller */
 +};
 +
  static const char *valid_cpus[] = {
  "cortex-a15",
  "cortex-a53",
 @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState 
 *vms, qemu_irq *pic)
  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
  hwaddr base = base_mmio;
 +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
 STREAM_ID_RANGE_SIZE;
>>> msi-base?
>>> STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
  int irq = vms->irqmap[VIRT_PCIE];
  MemoryRegion *mmio_alias;
 @@ -992,6 +1011,7 @@ static void create_pcie(const

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-09-01 Thread Diana Madalina Craciun 
On 08/22/2017 10:04 PM, Michael S. Tsirkin wrote:
> On Tue, Aug 22, 2017 at 03:13:57PM +, Diana Madalina Craciun wrote:
>> On 08/11/2017 06:50 PM, Edgar E. Iglesias wrote:
>>> On Fri, Aug 11, 2017 at 02:35:28PM +, Diana Madalina Craciun wrote:
 Hi Edgar,

 On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
>> Hi Diana,
>> On 23/05/2017 13:12, Diana Craciun wrote:
>>> Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
>>> Currently, for PCI devices, the requester ID was used as device
>>> ID in the virt machine. If the system has multiple masters that
>> if the system has multiple root complex?
>>> use MSIs a unique ID accross the platform is needed.
>> across
>>> A static scheme is used and each master is allocated a range of IDs
>>> with the formula:
>>> DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
>>> recommended by SBSA).
>>>
>>> This ID will be configured in the machine creation and if not configured
>>> the PCI requester ID will be used insteead.
>> instead
>>> Signed-off-by: Diana Craciun 
>>> ---
>>>  hw/arm/virt.c  | 26 ++
>>>  hw/pci-host/gpex.c |  6 ++
>>>  hw/pci/msi.c   |  2 +-
>>>  hw/pci/pci.c   | 25 +
>>>  include/hw/arm/virt.h  |  1 +
>>>  include/hw/pci-host/gpex.h |  2 ++
>>>  include/hw/pci/pci.h   |  8 
>>>  kvm-all.c  |  4 ++--
>>>  8 files changed, 71 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 5f62a03..a969694 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams 
>>> platform_bus_params;
>>>  #define RAMLIMIT_GB 255
>>>  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
>>>  
>>> +#define STREAM_ID_RANGE_SIZE 0x1
>>> +
>>>  /* Addresses and sizes of our components.
>>>   * 0..128MB is space for a flash device so we can run bootrom code 
>>> such as UEFI.
>>>   * 128MB..256MB is used for miscellaneous device I/O.
>>> @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
>>>  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 
>>> */
>>>  };
>>>  
>>> +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
>>> Currently
>>> + * for PCI devices the requester ID was used as device ID. But if the 
>>> system has
>>> + * multiple masters that use MSIs, the requester ID may cause deviceID 
>>> clashes.
>>> + * So a unique number is  needed accross the system.
>>> + * We are using the following formula:
>>> + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
>>> + * (as recommanded by SBSA). Currently we do not have an SMMU 
>>> emulation, but the
>>> + * same formula can be used for the generation of the streamID as well.
>>> + * For each master the device ID will be derrived from the requester 
>>> ID using
>>> + * the abovemntione formula.
>>> + */
>> I think most of this comment should only be in the commit message. typos
>> in derived and above mentioned.
>>
>> stream id is the terminology for the id space at the input of the smmu.
>> device id is the terminology for the id space at the input of the msi
>> controller I think.
>>
>> RID -> deviceID (no IOMMU)
>> RID -> streamID -> deviceID (IOMMU)
>>
>> I would personally get rid of all streamid uses as the smmu is not yet
>> supported and stick to the
>> Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
>>
>>> +
>>> +static const uint32_t streamidmap[] = {
>>> +[VIRT_PCIE] = 0, /* currently only one PCI controller */
>>> +};
>>> +
>>>  static const char *valid_cpus[] = {
>>>  "cortex-a15",
>>>  "cortex-a53",
>>> @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState 
>>> *vms, qemu_irq *pic)
>>>  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
>>>  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
>>>  hwaddr base = base_mmio;
>>> +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
>>> STREAM_ID_RANGE_SIZE;
>> msi-base?
>> STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
>>>  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
>>>  int irq = vms->irqmap[VIRT_PCIE];
>>>  MemoryRegion *mmio_alias;
>>> @@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState 
>>> *vms, qemu_irq *pic)
>>>  PCIHostState *pci;
>>>  
>>>  dev = qdev_create(NULL, TYPE_GPEX_HOST);
>>> +qdev_prop_set_uint32(dev,

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-08-23 Thread Edgar E. Iglesias 
On Tue, Aug 22, 2017 at 10:04:25PM +0300, Michael S. Tsirkin wrote:
> On Tue, Aug 22, 2017 at 03:13:57PM +, Diana Madalina Craciun wrote:
> > On 08/11/2017 06:50 PM, Edgar E. Iglesias wrote:
> > > On Fri, Aug 11, 2017 at 02:35:28PM +, Diana Madalina Craciun wrote:
> > >> Hi Edgar,
> > >>
> > >> On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
> > >>> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
> >  Hi Diana,
> >  On 23/05/2017 13:12, Diana Craciun wrote:
> > > Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
> > > Currently, for PCI devices, the requester ID was used as device
> > > ID in the virt machine. If the system has multiple masters that
> >  if the system has multiple root complex?
> > > use MSIs a unique ID accross the platform is needed.
> >  across
> > > A static scheme is used and each master is allocated a range of IDs
> > > with the formula:
> > > DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
> > > recommended by SBSA).
> > >
> > > This ID will be configured in the machine creation and if not 
> > > configured
> > > the PCI requester ID will be used insteead.
> >  instead
> > > Signed-off-by: Diana Craciun 
> > > ---
> > >  hw/arm/virt.c  | 26 ++
> > >  hw/pci-host/gpex.c |  6 ++
> > >  hw/pci/msi.c   |  2 +-
> > >  hw/pci/pci.c   | 25 +
> > >  include/hw/arm/virt.h  |  1 +
> > >  include/hw/pci-host/gpex.h |  2 ++
> > >  include/hw/pci/pci.h   |  8 
> > >  kvm-all.c  |  4 ++--
> > >  8 files changed, 71 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 5f62a03..a969694 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams 
> > > platform_bus_params;
> > >  #define RAMLIMIT_GB 255
> > >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
> > >  
> > > +#define STREAM_ID_RANGE_SIZE 0x1
> > > +
> > >  /* Addresses and sizes of our components.
> > >   * 0..128MB is space for a flash device so we can run bootrom code 
> > > such as UEFI.
> > >   * 128MB..256MB is used for miscellaneous device I/O.
> > > @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
> > >  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS 
> > > -1 */
> > >  };
> > >  
> > > +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
> > > Currently
> > > + * for PCI devices the requester ID was used as device ID. But if 
> > > the system has
> > > + * multiple masters that use MSIs, the requester ID may cause 
> > > deviceID clashes.
> > > + * So a unique number is  needed accross the system.
> > > + * We are using the following formula:
> > > + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
> > > + * (as recommanded by SBSA). Currently we do not have an SMMU 
> > > emulation, but the
> > > + * same formula can be used for the generation of the streamID as 
> > > well.
> > > + * For each master the device ID will be derrived from the requester 
> > > ID using
> > > + * the abovemntione formula.
> > > + */
> >  I think most of this comment should only be in the commit message. 
> >  typos
> >  in derived and above mentioned.
> > 
> >  stream id is the terminology for the id space at the input of the smmu.
> >  device id is the terminology for the id space at the input of the msi
> >  controller I think.
> > 
> >  RID -> deviceID (no IOMMU)
> >  RID -> streamID -> deviceID (IOMMU)
> > 
> >  I would personally get rid of all streamid uses as the smmu is not yet
> >  supported and stick to the
> >  Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
> > 
> > > +
> > > +static const uint32_t streamidmap[] = {
> > > +[VIRT_PCIE] = 0, /* currently only one PCI controller */
> > > +};
> > > +
> > >  static const char *valid_cpus[] = {
> > >  "cortex-a15",
> > >  "cortex-a53",
> > > @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState 
> > > *vms, qemu_irq *pic)
> > >  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
> > >  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
> > >  hwaddr base = base_mmio;
> > > +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
> > > STREAM_ID_RANGE_SIZE;
> >  msi-base?
> >  STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
> > >  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> > >  int irq = vms->irqmap[VIRT_PCIE];
> > >  MemoryRegion *mmio_alias;

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-08-22 Thread Michael S. Tsirkin 
On Tue, Aug 22, 2017 at 03:13:57PM +, Diana Madalina Craciun wrote:
> On 08/11/2017 06:50 PM, Edgar E. Iglesias wrote:
> > On Fri, Aug 11, 2017 at 02:35:28PM +, Diana Madalina Craciun wrote:
> >> Hi Edgar,
> >>
> >> On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
> >>> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
>  Hi Diana,
>  On 23/05/2017 13:12, Diana Craciun wrote:
> > Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
> > Currently, for PCI devices, the requester ID was used as device
> > ID in the virt machine. If the system has multiple masters that
>  if the system has multiple root complex?
> > use MSIs a unique ID accross the platform is needed.
>  across
> > A static scheme is used and each master is allocated a range of IDs
> > with the formula:
> > DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
> > recommended by SBSA).
> >
> > This ID will be configured in the machine creation and if not configured
> > the PCI requester ID will be used insteead.
>  instead
> > Signed-off-by: Diana Craciun 
> > ---
> >  hw/arm/virt.c  | 26 ++
> >  hw/pci-host/gpex.c |  6 ++
> >  hw/pci/msi.c   |  2 +-
> >  hw/pci/pci.c   | 25 +
> >  include/hw/arm/virt.h  |  1 +
> >  include/hw/pci-host/gpex.h |  2 ++
> >  include/hw/pci/pci.h   |  8 
> >  kvm-all.c  |  4 ++--
> >  8 files changed, 71 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 5f62a03..a969694 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams 
> > platform_bus_params;
> >  #define RAMLIMIT_GB 255
> >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
> >  
> > +#define STREAM_ID_RANGE_SIZE 0x1
> > +
> >  /* Addresses and sizes of our components.
> >   * 0..128MB is space for a flash device so we can run bootrom code 
> > such as UEFI.
> >   * 128MB..256MB is used for miscellaneous device I/O.
> > @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
> >  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 
> > */
> >  };
> >  
> > +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
> > Currently
> > + * for PCI devices the requester ID was used as device ID. But if the 
> > system has
> > + * multiple masters that use MSIs, the requester ID may cause deviceID 
> > clashes.
> > + * So a unique number is  needed accross the system.
> > + * We are using the following formula:
> > + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
> > + * (as recommanded by SBSA). Currently we do not have an SMMU 
> > emulation, but the
> > + * same formula can be used for the generation of the streamID as well.
> > + * For each master the device ID will be derrived from the requester 
> > ID using
> > + * the abovemntione formula.
> > + */
>  I think most of this comment should only be in the commit message. typos
>  in derived and above mentioned.
> 
>  stream id is the terminology for the id space at the input of the smmu.
>  device id is the terminology for the id space at the input of the msi
>  controller I think.
> 
>  RID -> deviceID (no IOMMU)
>  RID -> streamID -> deviceID (IOMMU)
> 
>  I would personally get rid of all streamid uses as the smmu is not yet
>  supported and stick to the
>  Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
> 
> > +
> > +static const uint32_t streamidmap[] = {
> > +[VIRT_PCIE] = 0, /* currently only one PCI controller */
> > +};
> > +
> >  static const char *valid_cpus[] = {
> >  "cortex-a15",
> >  "cortex-a53",
> > @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState 
> > *vms, qemu_irq *pic)
> >  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
> >  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
> >  hwaddr base = base_mmio;
> > +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
> > STREAM_ID_RANGE_SIZE;
>  msi-base?
>  STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
> >  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> >  int irq = vms->irqmap[VIRT_PCIE];
> >  MemoryRegion *mmio_alias;
> > @@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState 
> > *vms, qemu_irq *pic)
> >  PCIHostState *pci;
> >  
> >  dev = qdev_create(NULL, TYPE_GPEX_HOST);
> > +qdev_prop_set_uint32(dev, "stream-id-base", stream_id);
> >  qdev_init_nofail(dev);
>

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-08-22 Thread Diana Madalina Craciun 
On 08/11/2017 06:50 PM, Edgar E. Iglesias wrote:
> On Fri, Aug 11, 2017 at 02:35:28PM +, Diana Madalina Craciun wrote:
>> Hi Edgar,
>>
>> On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
>>> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
 Hi Diana,
 On 23/05/2017 13:12, Diana Craciun wrote:
> Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
> Currently, for PCI devices, the requester ID was used as device
> ID in the virt machine. If the system has multiple masters that
 if the system has multiple root complex?
> use MSIs a unique ID accross the platform is needed.
 across
> A static scheme is used and each master is allocated a range of IDs
> with the formula:
> DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
> recommended by SBSA).
>
> This ID will be configured in the machine creation and if not configured
> the PCI requester ID will be used insteead.
 instead
> Signed-off-by: Diana Craciun 
> ---
>  hw/arm/virt.c  | 26 ++
>  hw/pci-host/gpex.c |  6 ++
>  hw/pci/msi.c   |  2 +-
>  hw/pci/pci.c   | 25 +
>  include/hw/arm/virt.h  |  1 +
>  include/hw/pci-host/gpex.h |  2 ++
>  include/hw/pci/pci.h   |  8 
>  kvm-all.c  |  4 ++--
>  8 files changed, 71 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 5f62a03..a969694 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams platform_bus_params;
>  #define RAMLIMIT_GB 255
>  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
>  
> +#define STREAM_ID_RANGE_SIZE 0x1
> +
>  /* Addresses and sizes of our components.
>   * 0..128MB is space for a flash device so we can run bootrom code such 
> as UEFI.
>   * 128MB..256MB is used for miscellaneous device I/O.
> @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
>  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>  };
>  
> +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
> Currently
> + * for PCI devices the requester ID was used as device ID. But if the 
> system has
> + * multiple masters that use MSIs, the requester ID may cause deviceID 
> clashes.
> + * So a unique number is  needed accross the system.
> + * We are using the following formula:
> + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
> + * (as recommanded by SBSA). Currently we do not have an SMMU emulation, 
> but the
> + * same formula can be used for the generation of the streamID as well.
> + * For each master the device ID will be derrived from the requester ID 
> using
> + * the abovemntione formula.
> + */
 I think most of this comment should only be in the commit message. typos
 in derived and above mentioned.

 stream id is the terminology for the id space at the input of the smmu.
 device id is the terminology for the id space at the input of the msi
 controller I think.

 RID -> deviceID (no IOMMU)
 RID -> streamID -> deviceID (IOMMU)

 I would personally get rid of all streamid uses as the smmu is not yet
 supported and stick to the
 Documentation/devicetree/bindings/pci/pci-msi.txt terminology?

> +
> +static const uint32_t streamidmap[] = {
> +[VIRT_PCIE] = 0, /* currently only one PCI controller */
> +};
> +
>  static const char *valid_cpus[] = {
>  "cortex-a15",
>  "cortex-a53",
> @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState *vms, 
> qemu_irq *pic)
>  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
>  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
>  hwaddr base = base_mmio;
> +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
> STREAM_ID_RANGE_SIZE;
 msi-base?
 STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
>  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
>  int irq = vms->irqmap[VIRT_PCIE];
>  MemoryRegion *mmio_alias;
> @@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState *vms, 
> qemu_irq *pic)
>  PCIHostState *pci;
>  
>  dev = qdev_create(NULL, TYPE_GPEX_HOST);
> +qdev_prop_set_uint32(dev, "stream-id-base", stream_id);
>  qdev_init_nofail(dev);
>  
>  /* Map only the first size_ecam bytes of ECAM space */
> @@ -1056,6 +1076,11 @@ static void create_pcie(const VirtMachineState 
> *vms, qemu_irq *pic)
>  if (vms->msi_phandle) {
>  qemu_fdt_setprop_cells(vms->fdt, nodename, "msi-parent",
>

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-08-11 Thread Edgar E. Iglesias 
On Fri, Aug 11, 2017 at 02:35:28PM +, Diana Madalina Craciun wrote:
> Hi Edgar,
> 
> On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
> > On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
> >> Hi Diana,
> >> On 23/05/2017 13:12, Diana Craciun wrote:
> >>> Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
> >>> Currently, for PCI devices, the requester ID was used as device
> >>> ID in the virt machine. If the system has multiple masters that
> >> if the system has multiple root complex?
> >>> use MSIs a unique ID accross the platform is needed.
> >> across
> >>> A static scheme is used and each master is allocated a range of IDs
> >>> with the formula:
> >>> DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
> >>> recommended by SBSA).
> >>>
> >>> This ID will be configured in the machine creation and if not configured
> >>> the PCI requester ID will be used insteead.
> >> instead
> >>> Signed-off-by: Diana Craciun 
> >>> ---
> >>>  hw/arm/virt.c  | 26 ++
> >>>  hw/pci-host/gpex.c |  6 ++
> >>>  hw/pci/msi.c   |  2 +-
> >>>  hw/pci/pci.c   | 25 +
> >>>  include/hw/arm/virt.h  |  1 +
> >>>  include/hw/pci-host/gpex.h |  2 ++
> >>>  include/hw/pci/pci.h   |  8 
> >>>  kvm-all.c  |  4 ++--
> >>>  8 files changed, 71 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >>> index 5f62a03..a969694 100644
> >>> --- a/hw/arm/virt.c
> >>> +++ b/hw/arm/virt.c
> >>> @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams platform_bus_params;
> >>>  #define RAMLIMIT_GB 255
> >>>  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
> >>>  
> >>> +#define STREAM_ID_RANGE_SIZE 0x1
> >>> +
> >>>  /* Addresses and sizes of our components.
> >>>   * 0..128MB is space for a flash device so we can run bootrom code such 
> >>> as UEFI.
> >>>   * 128MB..256MB is used for miscellaneous device I/O.
> >>> @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
> >>>  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> >>>  };
> >>>  
> >>> +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
> >>> Currently
> >>> + * for PCI devices the requester ID was used as device ID. But if the 
> >>> system has
> >>> + * multiple masters that use MSIs, the requester ID may cause deviceID 
> >>> clashes.
> >>> + * So a unique number is  needed accross the system.
> >>> + * We are using the following formula:
> >>> + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
> >>> + * (as recommanded by SBSA). Currently we do not have an SMMU emulation, 
> >>> but the
> >>> + * same formula can be used for the generation of the streamID as well.
> >>> + * For each master the device ID will be derrived from the requester ID 
> >>> using
> >>> + * the abovemntione formula.
> >>> + */
> >> I think most of this comment should only be in the commit message. typos
> >> in derived and above mentioned.
> >>
> >> stream id is the terminology for the id space at the input of the smmu.
> >> device id is the terminology for the id space at the input of the msi
> >> controller I think.
> >>
> >> RID -> deviceID (no IOMMU)
> >> RID -> streamID -> deviceID (IOMMU)
> >>
> >> I would personally get rid of all streamid uses as the smmu is not yet
> >> supported and stick to the
> >> Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
> >>
> >>> +
> >>> +static const uint32_t streamidmap[] = {
> >>> +[VIRT_PCIE] = 0, /* currently only one PCI controller */
> >>> +};
> >>> +
> >>>  static const char *valid_cpus[] = {
> >>>  "cortex-a15",
> >>>  "cortex-a53",
> >>> @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState *vms, 
> >>> qemu_irq *pic)
> >>>  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
> >>>  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
> >>>  hwaddr base = base_mmio;
> >>> +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
> >>> STREAM_ID_RANGE_SIZE;
> >> msi-base?
> >> STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
> >>>  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> >>>  int irq = vms->irqmap[VIRT_PCIE];
> >>>  MemoryRegion *mmio_alias;
> >>> @@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState *vms, 
> >>> qemu_irq *pic)
> >>>  PCIHostState *pci;
> >>>  
> >>>  dev = qdev_create(NULL, TYPE_GPEX_HOST);
> >>> +qdev_prop_set_uint32(dev, "stream-id-base", stream_id);
> >>>  qdev_init_nofail(dev);
> >>>  
> >>>  /* Map only the first size_ecam bytes of ECAM space */
> >>> @@ -1056,6 +1076,11 @@ static void create_pcie(const VirtMachineState 
> >>> *vms, qemu_irq *pic)
> >>>  if (vms->msi_phandle) {
> >>>  qemu_fdt_setprop_cells(vms->fdt, nodename, "msi-parent",
> >>> vms->msi_phandle);
> >>> +

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-08-11 Thread Diana Madalina Craciun 
Hi Eric,

Thanks for looking into this.

On 07/26/2017 03:22 PM, Auger Eric wrote:

Hi Diana,
On 23/05/2017 13:12, Diana Craciun wrote:


Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
Currently, for PCI devices, the requester ID was used as device
ID in the virt machine. If the system has multiple masters that


if the system has multiple root complex?

Well ... root complex is PCI specific terminology. Our device is not a PCI 
device. However masters is not the best choice either, it should be rephrased.





use MSIs a unique ID accross the platform is needed.


across


A static scheme is used and each master is allocated a range of IDs
with the formula:
DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
recommended by SBSA).

This ID will be configured in the machine creation and if not configured
the PCI requester ID will be used insteead.


instead



Signed-off-by: Diana Craciun 

---
 hw/arm/virt.c  | 26 ++
 hw/pci-host/gpex.c |  6 ++
 hw/pci/msi.c   |  2 +-
 hw/pci/pci.c   | 25 +
 include/hw/arm/virt.h  |  1 +
 include/hw/pci-host/gpex.h |  2 ++
 include/hw/pci/pci.h   |  8 
 kvm-all.c  |  4 ++--
 8 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 5f62a03..a969694 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams platform_bus_params;
 #define RAMLIMIT_GB 255
 #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)

+#define STREAM_ID_RANGE_SIZE 0x1
+
 /* Addresses and sizes of our components.
  * 0..128MB is space for a flash device so we can run bootrom code such as 
UEFI.
  * 128MB..256MB is used for miscellaneous device I/O.
@@ -162,6 +164,22 @@ static const int a15irqmap[] = {
 [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
 };

+/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. Currently
+ * for PCI devices the requester ID was used as device ID. But if the system 
has
+ * multiple masters that use MSIs, the requester ID may cause deviceID clashes.
+ * So a unique number is  needed accross the system.
+ * We are using the following formula:
+ * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
+ * (as recommanded by SBSA). Currently we do not have an SMMU emulation, but 
the
+ * same formula can be used for the generation of the streamID as well.
+ * For each master the device ID will be derrived from the requester ID using
+ * the abovemntione formula.
+ */


I think most of this comment should only be in the commit message. typos
in derived and above mentioned.

OK.




stream id is the terminology for the id space at the input of the smmu.
device id is the terminology for the id space at the input of the msi
controller I think.

RID -> deviceID (no IOMMU)
RID -> streamID -> deviceID (IOMMU)

I would personally get rid of all streamid uses as the smmu is not yet
supported and stick to the
Documentation/devicetree/bindings/pci/pci-msi.txt terminology?


OK.





+
+static const uint32_t streamidmap[] = {
+[VIRT_PCIE] = 0, /* currently only one PCI controller */
+};
+
 static const char *valid_cpus[] = {
 "cortex-a15",
 "cortex-a53",
@@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState *vms, 
qemu_irq *pic)
 hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
 hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
 hwaddr base = base_mmio;
+uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * STREAM_ID_RANGE_SIZE;


msi-base?

OK, I will get rid of the stream_id naming.



STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?

OK.





 int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
 int irq = vms->irqmap[VIRT_PCIE];
 MemoryRegion *mmio_alias;
@@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState *vms, 
qemu_irq *pic)
 PCIHostState *pci;

 dev = qdev_create(NULL, TYPE_GPEX_HOST);
+qdev_prop_set_uint32(dev, "stream-id-base", stream_id);
 qdev_init_nofail(dev);

 /* Map only the first size_ecam bytes of ECAM space */
@@ -1056,6 +1076,11 @@ static void create_pcie(const VirtMachineState *vms, 
qemu_irq *pic)
 if (vms->msi_phandle) {
 qemu_fdt_setprop_cells(vms->fdt, nodename, "msi-parent",
vms->msi_phandle);
+qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "msi-map",
+ 1, 0,
+ 1, vms->msi_phandle,
+ 1, stream_id,
+ 1, STREAM_ID_RANGE_SIZE);
 }

 qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
@@ -1609,6 +1634,7 @@ static void virt_2_9_instance_init(Object *obj)

 vms->memmap = a15memmap;
 vms->irqmap = a15irqmap;
+

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-08-11 Thread Diana Madalina Craciun 
Hi Edgar,

On 07/31/2017 06:16 PM, Edgar E. Iglesias wrote:
> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
>> Hi Diana,
>> On 23/05/2017 13:12, Diana Craciun wrote:
>>> Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
>>> Currently, for PCI devices, the requester ID was used as device
>>> ID in the virt machine. If the system has multiple masters that
>> if the system has multiple root complex?
>>> use MSIs a unique ID accross the platform is needed.
>> across
>>> A static scheme is used and each master is allocated a range of IDs
>>> with the formula:
>>> DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
>>> recommended by SBSA).
>>>
>>> This ID will be configured in the machine creation and if not configured
>>> the PCI requester ID will be used insteead.
>> instead
>>> Signed-off-by: Diana Craciun 
>>> ---
>>>  hw/arm/virt.c  | 26 ++
>>>  hw/pci-host/gpex.c |  6 ++
>>>  hw/pci/msi.c   |  2 +-
>>>  hw/pci/pci.c   | 25 +
>>>  include/hw/arm/virt.h  |  1 +
>>>  include/hw/pci-host/gpex.h |  2 ++
>>>  include/hw/pci/pci.h   |  8 
>>>  kvm-all.c  |  4 ++--
>>>  8 files changed, 71 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 5f62a03..a969694 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams platform_bus_params;
>>>  #define RAMLIMIT_GB 255
>>>  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
>>>  
>>> +#define STREAM_ID_RANGE_SIZE 0x1
>>> +
>>>  /* Addresses and sizes of our components.
>>>   * 0..128MB is space for a flash device so we can run bootrom code such as 
>>> UEFI.
>>>   * 128MB..256MB is used for miscellaneous device I/O.
>>> @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
>>>  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>>>  };
>>>  
>>> +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
>>> Currently
>>> + * for PCI devices the requester ID was used as device ID. But if the 
>>> system has
>>> + * multiple masters that use MSIs, the requester ID may cause deviceID 
>>> clashes.
>>> + * So a unique number is  needed accross the system.
>>> + * We are using the following formula:
>>> + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
>>> + * (as recommanded by SBSA). Currently we do not have an SMMU emulation, 
>>> but the
>>> + * same formula can be used for the generation of the streamID as well.
>>> + * For each master the device ID will be derrived from the requester ID 
>>> using
>>> + * the abovemntione formula.
>>> + */
>> I think most of this comment should only be in the commit message. typos
>> in derived and above mentioned.
>>
>> stream id is the terminology for the id space at the input of the smmu.
>> device id is the terminology for the id space at the input of the msi
>> controller I think.
>>
>> RID -> deviceID (no IOMMU)
>> RID -> streamID -> deviceID (IOMMU)
>>
>> I would personally get rid of all streamid uses as the smmu is not yet
>> supported and stick to the
>> Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
>>
>>> +
>>> +static const uint32_t streamidmap[] = {
>>> +[VIRT_PCIE] = 0, /* currently only one PCI controller */
>>> +};
>>> +
>>>  static const char *valid_cpus[] = {
>>>  "cortex-a15",
>>>  "cortex-a53",
>>> @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState *vms, 
>>> qemu_irq *pic)
>>>  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
>>>  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
>>>  hwaddr base = base_mmio;
>>> +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
>>> STREAM_ID_RANGE_SIZE;
>> msi-base?
>> STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
>>>  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
>>>  int irq = vms->irqmap[VIRT_PCIE];
>>>  MemoryRegion *mmio_alias;
>>> @@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState *vms, 
>>> qemu_irq *pic)
>>>  PCIHostState *pci;
>>>  
>>>  dev = qdev_create(NULL, TYPE_GPEX_HOST);
>>> +qdev_prop_set_uint32(dev, "stream-id-base", stream_id);
>>>  qdev_init_nofail(dev);
>>>  
>>>  /* Map only the first size_ecam bytes of ECAM space */
>>> @@ -1056,6 +1076,11 @@ static void create_pcie(const VirtMachineState *vms, 
>>> qemu_irq *pic)
>>>  if (vms->msi_phandle) {
>>>  qemu_fdt_setprop_cells(vms->fdt, nodename, "msi-parent",
>>> vms->msi_phandle);
>>> +qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "msi-map",
>>> + 1, 0,
>>> + 1, vms->msi_phandle,
>>> + 1, stream_id,
>>> + 1, STREAM_ID_RANGE_SIZE);
>>>  }
>>>

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-07-31 Thread Edgar E. Iglesias 
On Mon, Jul 31, 2017 at 05:16:02PM +0200, Edgar E. Iglesias wrote:
> On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
> > Hi Diana,
> > On 23/05/2017 13:12, Diana Craciun wrote:
> > > Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
> > > Currently, for PCI devices, the requester ID was used as device
> > > ID in the virt machine. If the system has multiple masters that
> > if the system has multiple root complex?
> > > use MSIs a unique ID accross the platform is needed.
> > across
> > > A static scheme is used and each master is allocated a range of IDs
> > > with the formula:
> > > DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
> > > recommended by SBSA).
> > > 
> > > This ID will be configured in the machine creation and if not configured
> > > the PCI requester ID will be used insteead.
> > instead
> > > 
> > > Signed-off-by: Diana Craciun 
> > > ---
> > >  hw/arm/virt.c  | 26 ++
> > >  hw/pci-host/gpex.c |  6 ++
> > >  hw/pci/msi.c   |  2 +-
> > >  hw/pci/pci.c   | 25 +
> > >  include/hw/arm/virt.h  |  1 +
> > >  include/hw/pci-host/gpex.h |  2 ++
> > >  include/hw/pci/pci.h   |  8 
> > >  kvm-all.c  |  4 ++--
> > >  8 files changed, 71 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > > index 5f62a03..a969694 100644
> > > --- a/hw/arm/virt.c
> > > +++ b/hw/arm/virt.c
> > > @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams platform_bus_params;
> > >  #define RAMLIMIT_GB 255
> > >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
> > >  
> > > +#define STREAM_ID_RANGE_SIZE 0x1
> > > +
> > >  /* Addresses and sizes of our components.
> > >   * 0..128MB is space for a flash device so we can run bootrom code such 
> > > as UEFI.
> > >   * 128MB..256MB is used for miscellaneous device I/O.
> > > @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
> > >  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> > >  };
> > >  
> > > +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
> > > Currently
> > > + * for PCI devices the requester ID was used as device ID. But if the 
> > > system has
> > > + * multiple masters that use MSIs, the requester ID may cause deviceID 
> > > clashes.
> > > + * So a unique number is  needed accross the system.
> > > + * We are using the following formula:
> > > + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
> > > + * (as recommanded by SBSA). Currently we do not have an SMMU emulation, 
> > > but the
> > > + * same formula can be used for the generation of the streamID as well.
> > > + * For each master the device ID will be derrived from the requester ID 
> > > using
> > > + * the abovemntione formula.
> > > + */
> > I think most of this comment should only be in the commit message. typos
> > in derived and above mentioned.
> > 
> > stream id is the terminology for the id space at the input of the smmu.
> > device id is the terminology for the id space at the input of the msi
> > controller I think.
> > 
> > RID -> deviceID (no IOMMU)
> > RID -> streamID -> deviceID (IOMMU)
> > 
> > I would personally get rid of all streamid uses as the smmu is not yet
> > supported and stick to the
> > Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
> > 
> > > +
> > > +static const uint32_t streamidmap[] = {
> > > +[VIRT_PCIE] = 0, /* currently only one PCI controller */
> > > +};
> > > +
> > >  static const char *valid_cpus[] = {
> > >  "cortex-a15",
> > >  "cortex-a53",
> > > @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState *vms, 
> > > qemu_irq *pic)
> > >  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
> > >  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
> > >  hwaddr base = base_mmio;
> > > +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
> > > STREAM_ID_RANGE_SIZE;
> > msi-base?
> > STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
> > >  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> > >  int irq = vms->irqmap[VIRT_PCIE];
> > >  MemoryRegion *mmio_alias;
> > > @@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState *vms, 
> > > qemu_irq *pic)
> > >  PCIHostState *pci;
> > >  
> > >  dev = qdev_create(NULL, TYPE_GPEX_HOST);
> > > +qdev_prop_set_uint32(dev, "stream-id-base", stream_id);
> > >  qdev_init_nofail(dev);
> > >  
> > >  /* Map only the first size_ecam bytes of ECAM space */
> > > @@ -1056,6 +1076,11 @@ static void create_pcie(const VirtMachineState 
> > > *vms, qemu_irq *pic)
> > >  if (vms->msi_phandle) {
> > >  qemu_fdt_setprop_cells(vms->fdt, nodename, "msi-parent",
> > > vms->msi_phandle);
> > > +qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "msi-map",
> > > +

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-07-31 Thread Edgar E. Iglesias 
On Wed, Jul 26, 2017 at 02:22:28PM +0200, Auger Eric wrote:
> Hi Diana,
> On 23/05/2017 13:12, Diana Craciun wrote:
> > Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
> > Currently, for PCI devices, the requester ID was used as device
> > ID in the virt machine. If the system has multiple masters that
> if the system has multiple root complex?
> > use MSIs a unique ID accross the platform is needed.
> across
> > A static scheme is used and each master is allocated a range of IDs
> > with the formula:
> > DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
> > recommended by SBSA).
> > 
> > This ID will be configured in the machine creation and if not configured
> > the PCI requester ID will be used insteead.
> instead
> > 
> > Signed-off-by: Diana Craciun 
> > ---
> >  hw/arm/virt.c  | 26 ++
> >  hw/pci-host/gpex.c |  6 ++
> >  hw/pci/msi.c   |  2 +-
> >  hw/pci/pci.c   | 25 +
> >  include/hw/arm/virt.h  |  1 +
> >  include/hw/pci-host/gpex.h |  2 ++
> >  include/hw/pci/pci.h   |  8 
> >  kvm-all.c  |  4 ++--
> >  8 files changed, 71 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 5f62a03..a969694 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams platform_bus_params;
> >  #define RAMLIMIT_GB 255
> >  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
> >  
> > +#define STREAM_ID_RANGE_SIZE 0x1
> > +
> >  /* Addresses and sizes of our components.
> >   * 0..128MB is space for a flash device so we can run bootrom code such as 
> > UEFI.
> >   * 128MB..256MB is used for miscellaneous device I/O.
> > @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
> >  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
> >  };
> >  
> > +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. 
> > Currently
> > + * for PCI devices the requester ID was used as device ID. But if the 
> > system has
> > + * multiple masters that use MSIs, the requester ID may cause deviceID 
> > clashes.
> > + * So a unique number is  needed accross the system.
> > + * We are using the following formula:
> > + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
> > + * (as recommanded by SBSA). Currently we do not have an SMMU emulation, 
> > but the
> > + * same formula can be used for the generation of the streamID as well.
> > + * For each master the device ID will be derrived from the requester ID 
> > using
> > + * the abovemntione formula.
> > + */
> I think most of this comment should only be in the commit message. typos
> in derived and above mentioned.
> 
> stream id is the terminology for the id space at the input of the smmu.
> device id is the terminology for the id space at the input of the msi
> controller I think.
> 
> RID -> deviceID (no IOMMU)
> RID -> streamID -> deviceID (IOMMU)
> 
> I would personally get rid of all streamid uses as the smmu is not yet
> supported and stick to the
> Documentation/devicetree/bindings/pci/pci-msi.txt terminology?
> 
> > +
> > +static const uint32_t streamidmap[] = {
> > +[VIRT_PCIE] = 0, /* currently only one PCI controller */
> > +};
> > +
> >  static const char *valid_cpus[] = {
> >  "cortex-a15",
> >  "cortex-a53",
> > @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState *vms, 
> > qemu_irq *pic)
> >  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
> >  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
> >  hwaddr base = base_mmio;
> > +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * 
> > STREAM_ID_RANGE_SIZE;
> msi-base?
> STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
> >  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
> >  int irq = vms->irqmap[VIRT_PCIE];
> >  MemoryRegion *mmio_alias;
> > @@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState *vms, 
> > qemu_irq *pic)
> >  PCIHostState *pci;
> >  
> >  dev = qdev_create(NULL, TYPE_GPEX_HOST);
> > +qdev_prop_set_uint32(dev, "stream-id-base", stream_id);
> >  qdev_init_nofail(dev);
> >  
> >  /* Map only the first size_ecam bytes of ECAM space */
> > @@ -1056,6 +1076,11 @@ static void create_pcie(const VirtMachineState *vms, 
> > qemu_irq *pic)
> >  if (vms->msi_phandle) {
> >  qemu_fdt_setprop_cells(vms->fdt, nodename, "msi-parent",
> > vms->msi_phandle);
> > +qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "msi-map",
> > + 1, 0,
> > + 1, vms->msi_phandle,
> > + 1, stream_id,
> > + 1, STREAM_ID_RANGE_SIZE);
> >  }
> >  
> >  qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
> >

Re: [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID

2017-07-26 Thread Auger Eric 
Hi Diana,
On 23/05/2017 13:12, Diana Craciun wrote:
> Device IDs are required by the ARM GICv3 ITS for IRQ remapping.
> Currently, for PCI devices, the requester ID was used as device
> ID in the virt machine. If the system has multiple masters that
if the system has multiple root complex?
> use MSIs a unique ID accross the platform is needed.
across
> A static scheme is used and each master is allocated a range of IDs
> with the formula:
> DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant (as
> recommended by SBSA).
> 
> This ID will be configured in the machine creation and if not configured
> the PCI requester ID will be used insteead.
instead
> 
> Signed-off-by: Diana Craciun 
> ---
>  hw/arm/virt.c  | 26 ++
>  hw/pci-host/gpex.c |  6 ++
>  hw/pci/msi.c   |  2 +-
>  hw/pci/pci.c   | 25 +
>  include/hw/arm/virt.h  |  1 +
>  include/hw/pci-host/gpex.h |  2 ++
>  include/hw/pci/pci.h   |  8 
>  kvm-all.c  |  4 ++--
>  8 files changed, 71 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 5f62a03..a969694 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -110,6 +110,8 @@ static ARMPlatformBusSystemParams platform_bus_params;
>  #define RAMLIMIT_GB 255
>  #define RAMLIMIT_BYTES (RAMLIMIT_GB * 1024ULL * 1024 * 1024)
>  
> +#define STREAM_ID_RANGE_SIZE 0x1
> +
>  /* Addresses and sizes of our components.
>   * 0..128MB is space for a flash device so we can run bootrom code such as 
> UEFI.
>   * 128MB..256MB is used for miscellaneous device I/O.
> @@ -162,6 +164,22 @@ static const int a15irqmap[] = {
>  [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 */
>  };
>  
> +/* Device IDs are required by the ARM GICV3 ITS for IRQ remapping. Currently
> + * for PCI devices the requester ID was used as device ID. But if the system 
> has
> + * multiple masters that use MSIs, the requester ID may cause deviceID 
> clashes.
> + * So a unique number is  needed accross the system.
> + * We are using the following formula:
> + * DeviceID = zero_extend( RequesterID[15:0] ) + 0x1*Constant
> + * (as recommanded by SBSA). Currently we do not have an SMMU emulation, but 
> the
> + * same formula can be used for the generation of the streamID as well.
> + * For each master the device ID will be derrived from the requester ID using
> + * the abovemntione formula.
> + */
I think most of this comment should only be in the commit message. typos
in derived and above mentioned.

stream id is the terminology for the id space at the input of the smmu.
device id is the terminology for the id space at the input of the msi
controller I think.

RID -> deviceID (no IOMMU)
RID -> streamID -> deviceID (IOMMU)

I would personally get rid of all streamid uses as the smmu is not yet
supported and stick to the
Documentation/devicetree/bindings/pci/pci-msi.txt terminology?

> +
> +static const uint32_t streamidmap[] = {
> +[VIRT_PCIE] = 0, /* currently only one PCI controller */
> +};
> +
>  static const char *valid_cpus[] = {
>  "cortex-a15",
>  "cortex-a53",
> @@ -980,6 +998,7 @@ static void create_pcie(const VirtMachineState *vms, 
> qemu_irq *pic)
>  hwaddr base_ecam = vms->memmap[VIRT_PCIE_ECAM].base;
>  hwaddr size_ecam = vms->memmap[VIRT_PCIE_ECAM].size;
>  hwaddr base = base_mmio;
> +uint32_t stream_id = vms->streamidmap[VIRT_PCIE] * STREAM_ID_RANGE_SIZE;
msi-base?
STREAM_ID_RANGE_SIZE ~ MSI_MAP_LENGTH?
>  int nr_pcie_buses = size_ecam / PCIE_MMCFG_SIZE_MIN;
>  int irq = vms->irqmap[VIRT_PCIE];
>  MemoryRegion *mmio_alias;
> @@ -992,6 +1011,7 @@ static void create_pcie(const VirtMachineState *vms, 
> qemu_irq *pic)
>  PCIHostState *pci;
>  
>  dev = qdev_create(NULL, TYPE_GPEX_HOST);
> +qdev_prop_set_uint32(dev, "stream-id-base", stream_id);
>  qdev_init_nofail(dev);
>  
>  /* Map only the first size_ecam bytes of ECAM space */
> @@ -1056,6 +1076,11 @@ static void create_pcie(const VirtMachineState *vms, 
> qemu_irq *pic)
>  if (vms->msi_phandle) {
>  qemu_fdt_setprop_cells(vms->fdt, nodename, "msi-parent",
> vms->msi_phandle);
> +qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "msi-map",
> + 1, 0,
> + 1, vms->msi_phandle,
> + 1, stream_id,
> + 1, STREAM_ID_RANGE_SIZE);
>  }
>  
>  qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
> @@ -1609,6 +1634,7 @@ static void virt_2_9_instance_init(Object *obj)
>  
>  vms->memmap = a15memmap;
>  vms->irqmap = a15irqmap;
> +vms->streamidmap = streamidmap;
>  }
>  
>  static void virt_machine_2_9_options(MachineClass *mc)
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index