qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt m


From: Michael S. Tsirkin
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH v2 2/2] Add a unique ID in the virt machine to be used as device ID
Date: Fri, 1 Sep 2017 18:32:32 +0300

On Fri, Sep 01, 2017 at 02:32:59PM +0000, 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 +0000, Diana Madalina Craciun wrote:
> >>> On 08/11/2017 06:50 PM, Edgar E. Iglesias wrote:
> >>>> On Fri, Aug 11, 2017 at 02:35:28PM +0000, 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] ) + 0x10000*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 <address@hidden>
> >>>>>>>> ---
> >>>>>>>>  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 0x10000
> >>>>>>>> +
> >>>>>>>>  /* 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] ) + 0x10000*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 66055ee..de72408 100644
> >>>>>>>> --- a/hw/pci-host/gpex.c
> >>>>>>>> +++ b/hw/pci-host/gpex.c
> >>>>>>>> @@ -43,6 +43,11 @@ static void gpex_set_irq(void *opaque, int 
> >>>>>>>> irq_num, int level)
> >>>>>>>>      qemu_set_irq(s->irq[irq_num], level);
> >>>>>>>>  }
> >>>>>>>>  
> >>>>>>>> +static Property gpex_props[] = {
> >>>>>>>> +    DEFINE_PROP_UINT32("stream-id-base", GPEXHost, stream_id_base, 
> >>>>>>>> 0),
> >>>>>>> msi_base_base
> >>>>>>>> +    DEFINE_PROP_END_OF_LIST(),
> >>>>>>>> +};
> >>>>>>>> +
> >>>>>>>>  static void gpex_host_realize(DeviceState *dev, Error **errp)
> >>>>>>>>  {
> >>>>>>>>      PCIHostState *pci = PCI_HOST_BRIDGE(dev);
> >>>>>>>> @@ -83,6 +88,7 @@ static void gpex_host_class_init(ObjectClass 
> >>>>>>>> *klass, void *data)
> >>>>>>>>  
> >>>>>>>>      hc->root_bus_path = gpex_host_root_bus_path;
> >>>>>>>>      dc->realize = gpex_host_realize;
> >>>>>>>> +    dc->props = gpex_props;
> >>>>>>>>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> >>>>>>>>      dc->fw_name = "pci";
> >>>>>>>>  }
> >>>>>>>> diff --git a/hw/pci/msi.c b/hw/pci/msi.c
> >>>>>>>> index 7925851..b60a410 100644
> >>>>>>>> --- a/hw/pci/msi.c
> >>>>>>>> +++ b/hw/pci/msi.c
> >>>>>>>> @@ -336,7 +336,7 @@ void msi_send_message(PCIDevice *dev, MSIMessage 
> >>>>>>>> msg)
> >>>>>>>>  {
> >>>>>>>>      MemTxAttrs attrs = {};
> >>>>>>>>  
> >>>>>>>> -    attrs.stream_id = pci_requester_id(dev);
> >>>>>>>> +    attrs.stream_id = pci_stream_id(dev);
> >>>>>>>>      address_space_stl_le(&dev->bus_master_as, msg.address, msg.data,
> >>>>>>>>                           attrs, NULL);
> >>>>>>>>  }
> >>>>>>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >>>>>>>> index 259483b..92e9a2b 100644
> >>>>>>>> --- a/hw/pci/pci.c
> >>>>>>>> +++ b/hw/pci/pci.c
> >>>>>>>> @@ -951,6 +951,30 @@ uint16_t pci_requester_id(PCIDevice *dev)
> >>>>>>>>      return pci_req_id_cache_extract(&dev->requester_id_cache);
> >>>>>>>>  }
> >>>>>>>>  
> >>>>>>>> +static uint32_t pci_get_stream_id_base(PCIDevice *dev)
> >>>>>>>> +{
> >>>>>>>> +    PCIBus *rootbus = pci_device_root_bus(dev);
> >>>>>>>> +    PCIHostState *host_bridge = 
> >>>>>>>> PCI_HOST_BRIDGE(rootbus->qbus.parent);
> >>>>>>>> +    Error *err = NULL;
> >>>>>>>> +    int64_t stream_id;
> >>>>>>>> +
> >>>>>>>> +    stream_id = object_property_get_int(OBJECT(host_bridge), 
> >>>>>>>> "stream-id-base",
> >>>>>>>> +                                        &err);
> >>>>>>>> +    if (stream_id < 0) {
> >>>>>>>> +        stream_id = 0;
> >>>>>>>> +    }
> >>>>>>>> +
> >>>>>>>> +    return stream_id;
> >>>>>>>> +}
> >>>>>>>> +
> >>>>>>>> +uint32_t pci_stream_id(PCIDevice *dev)
> >>>>>>>> +{
> >>>>>>>> +    /* Stream ID = RequesterID[15:0] + stream_id_base. 
> >>>>>>>> stream_id_base may
> >>>>>>>> +     * be 0 for devices that are not using any translation between 
> >>>>>>>> requester_id
> >>>>>>>> +     * and stream_id */
> >>>>>>>> +    return  (uint16_t)pci_requester_id(dev) + dev->stream_id_base;
> >>>>>>>> +}
> >>>>>>> I think you should split the changes in virt from pci/gpex generic 
> >>>>>>> changes.
> >>>>>>>
> >>>>>>>> +
> >>>>>>>>  /* -1 for devfn means auto assign */
> >>>>>>>>  static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus 
> >>>>>>>> *bus,
> >>>>>>>>                                           const char *name, int 
> >>>>>>>> devfn,
> >>>>>>>> @@ -1000,6 +1024,7 @@ static PCIDevice 
> >>>>>>>> *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
> >>>>>>>>  
> >>>>>>>>      pci_dev->devfn = devfn;
> >>>>>>>>      pci_dev->requester_id_cache = pci_req_id_cache_get(pci_dev);
> >>>>>>>> +    pci_dev->stream_id_base = pci_get_stream_id_base(pci_dev);
> >>>>>>> looks strange to me to store the rid base in the end point as this is
> >>>>>>> rather a property of the PCI complex. I acknowledge this is much more
> >>>>>> I agree.
> >>>>> The reason I have changed was to avoid traversing the entire hierarchy
> >>>>> each time the ID is needed (for example each time when a MSI is sent).
> >>>>>
> >>>>>> I think that what we need is to add support for allowing PCI RCs
> >>>>>> to transform requesterIDs in transactions attributes according to the
> >>>>>> implementation specifics.
> >>>>> Do you mean that you need more than a linear offset between requesterID
> >>>>> and whatever other ID?
> >>>> Yes.
> >>>>
> >>>> This is my understanding for the ARM platforms I'm familiar with:
> >>>>
> >>>> Since AXI busses don't have a defined way to carry Master IDs, these
> >>>> are typically carried on the AXI user signals. I'll just refer to
> >>>> these signals as AXI Master IDs.
> >>>>
> >>>> 1. An endpoint issues an MSI (or any) transaction on the PCI bus.
> >>>>    In QEMU, these trasactions carry the requester ID in their attributes.
> >>>>
> >>>> 2. The transaction hits the PCI "host" bridge to the SoC internal
> >>>>    interconnect (typically AXI). This bridge needs to forward the
> >>>>    PCI transaction onto the AXI bus. Including mapping the PCI
> >>>>    RequesterID into an AXI MasterID.
> >>>>
> >>>> 3. The AXI transaction hits the IOMMU and the MasterID is mapped
> >>>>    into a streamID to identify the origin of the transaction
> >>>>    and apply address translation accordingly. If the SMMU
> >>>>    allows the transaction to pass, the stream ID is mapped back
> >>>>    into the transactions MasterID.
> >>>>
> >>>> 4. The AXI transaction continues down the interconnect and hits
> >>>>    the MSI doorbell and the MasterID is mapped into a DeviceID to
> >>>>    identify the origin of the MSI and apply possible interrupt 
> >>>> translation.
> >>>>
> >>>> Adding streamID fields to a PCI endpoint doesn't make any sense to me.
> >>>> The requester ID is already there and is IMO enough.
> >>>> StreamIDs are a concept of ARM System MMUs, not of PCI endpoints.
> >>> What I have added into the endpoint is actually the Master ID (in QEMU
> >>> it is actually equal with the streamID). I agree that this is a property
> >>> of the root complex, the only reason I have put it into the endpoint was
> >>> to avoid traversing the PCI hierarchy each time an MSI is sent.
> >> Can all this be folded into the IOMMU? Then you might be able to get by
> >> with defining an iommu function.  pci_device_iommu_address_space already
> >> walks the hierarchy.
> > Hmm, perhaps. I guess iommu_fn's would have to be able to return modified
> > transaction attributes. That would work, I think,
> >
> > I was first thinking that if we change the IOMMU translate() method to allow
> > IOMMUs to modify memory attributes, Diana could register an IOMMU 
> > memory-region
> > with pci_setup_iommu() that modifies the attributes and returns a new 
> > attribute
> > with the AS that originally would have been set with pci_setup_iommu().
> >
> > Adding support for IOMMU translate() to modify attributes is something we
> > need to do anyway IMO.
> >
> >
> >>>> When modelling #2, hardcoding a specific linear mapping between
> >>>> PCI requester IDs and AXI Master IDs may work for one platform
> >>>> but it won't work for all platforms. There is no one mapping for all.
> >>>> It can even be run-time programmable in the bridge.
> >> OK but how does it work with the specific bridge that you emulate?
> >> There is no need to model advanced bridges with super flexible
> >> programmable mappings if guests do not need them to run.
> >
> > Diana can answer for the details of the bridge she wants to model.
> > I was mostly trying to make a point that we should avoid hardcoding
> > a specific mapping that cannot be overriden by other host bridge models.
> 
> I am using the generic host bridge from the virt machine, so nothing
> special. The problem I am trying to solve is having non-PCI devices
> which are using MSIs as well. So they need also a translation from
> whatever ID they are having to device ID. And the translated device ID
> must not be in the same range with the PCI ones.
> 
> 
> >
> >>> One solution might be defining a function in the generic host bridge
> >>> which by default returns the requesterIDs (assumes that requesterID is
> >>> the same with the masterID). This function can be over overridden by
> >>> each specific implementation.
> >>>
> >>>> IIRC, the SMMUv3 docs have a section that suggest how these ReqID to AXI 
> >>>> Master ID mappings can be done.
> >>> I did not find the specific section, just that the streamID should be
> >>> derived from requesterID.
> >
> > In my version of the spec, it's section 4.2 Stream Numbering.
> > At the end of that secion there are some recomendations on how to
> > map PCI ReqIDs into Stream IDs.
> 
> OK, I found that section. In that section ARM recommends the streamID to
> be generated from PCI requester ID so that StreamID[15:0] ==
> RequesterID[15:0]. It also mentions that in the case of more than one
> root complex to extend the number of streamID bits to differentiate
> between them (e.g. bits [N:16]).
> 
> Almost the same idea (a linear mapping) is presented in the SBSA spec:
> StreamID = RequesterID[15:0] + 0x10000 * Constant
> 
> My proposal is an implementation of the SBSA version.
> 
> In this email thread it has been suggested to use Constant=0 for PCI and
> leave the PCI code unmodified. It can be done this way, but this will
> not work in case of having multiple root complexes because we should
> also reserve some bits [N:16] to differentiate between the root
> complexes, but in the same time not to overlap with the platform devices
> pool.

Once we have multi-root properly on ARM/x86 yet, when we do specifying
the segment and then caching it on the the device might make sense.

For now just assume segment # is 0, let's not over-engineer.

> We can agree upon a number of bits reserved for PCIe segment and
> used the remaining ones for other platforms.

I'd set all high bits to 1 for non-PCI MSI as a first step.  When there
is more than one kind of non-PCI MSI we'll come up with another mapping.

> 
> > Cheers,
> > Edgar
> >
> >
> >
> >
> >>>
> >>> Thanks,
> >>>
> >>> Diana
> >>>>
> >>>>>> The way we did it when modelling the ZynqMP is by adding support for
> >>>>>> transaction attribute translation in QEMU's IOMMU interface.
> >>>>>> In our PCI RC, we have an IOMMU covering the entire AS that PCI devs 
> >>>>>> DMA into.
> >>>>>> This IOMMU doesn't do address-translation, only RequesterID -> StreamID
> >>>>>> transforms according to how the ZynqMP PCI RC derives StreamIDs from 
> >>>>>> RequesterIDs.
> >>>>> Are there any patches for this support in order for me to better 
> >>>>> understand?
> >>>> It's currently on the Xilinx QEMU fork on GitHub.
> >>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FXilinx%2Fqemu%2Fblob%2Fmaster%2Fhw%2Fpci-host%2Fxlnx-nwl-pcie-main.c&data=01%7C01%7Cdiana.craciun%40nxp.com%7C8db54b1ee05143ceefbf08d4ea65a585%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=h%2F4ZlhJbYkL0dRbxv7MykGuwlRTmV64GsK%2FhH7woBtE%3D&reserved=0
> >>>>
> >>>> In the current ZynqMP, all RequesterIDs map to a single MasterID (it's a 
> >>>> HW limitation).
> >>>> In future versions of the HW, another mapping will be used.
> >>>> I can't share code for the latter yet though....
> >>>>
> >>>> Best regards,
> >>>> Edgar
> >>>>
> >>>>  
> >>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Diana
> >>>>>
> >>>>>
> >>>>>> This is useful not only to model PCI RequesterID to AXI Master ID 
> >>>>>> mappings but
> >>>>>> also for modelling things like the ARM TZC (or the Xilinx ZynqMP 
> >>>>>> XMPU/XPPUs).
> >>>>>>
> >>>>>> Cheers,
> >>>>>> Edgar
> >>>>>>
> >>>>>>
> >>>>>>> simple than reworking pci_requester_id() though.
> >>>>>>>>  
> >>>>>>>>      memory_region_init(&pci_dev->bus_master_container_region, 
> >>>>>>>> OBJECT(pci_dev),
> >>>>>>>>                         "bus master container", UINT64_MAX);
> >>>>>>>> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> >>>>>>>> index 33b0ff3..94c007a 100644
> >>>>>>>> --- a/include/hw/arm/virt.h
> >>>>>>>> +++ b/include/hw/arm/virt.h
> >>>>>>>> @@ -99,6 +99,7 @@ typedef struct {
> >>>>>>>>      struct arm_boot_info bootinfo;
> >>>>>>>>      const MemMapEntry *memmap;
> >>>>>>>>      const int *irqmap;
> >>>>>>>> +    const uint32_t *streamidmap;
> >>>>>>>>      int smp_cpus;
> >>>>>>>>      void *fdt;
> >>>>>>>>      int fdt_size;
> >>>>>>>> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> >>>>>>>> index 68c9348..47df01a 100644
> >>>>>>>> --- a/include/hw/pci-host/gpex.h
> >>>>>>>> +++ b/include/hw/pci-host/gpex.h
> >>>>>>>> @@ -48,6 +48,8 @@ typedef struct GPEXHost {
> >>>>>>>>  
> >>>>>>>>      GPEXRootState gpex_root;
> >>>>>>>>  
> >>>>>>>> +    uint32_t stream_id_base;
> >>>>>>>> +
> >>>>>>>>      MemoryRegion io_ioport;
> >>>>>>>>      MemoryRegion io_mmio;
> >>>>>>>>      qemu_irq irq[GPEX_NUM_IRQS];
> >>>>>>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> >>>>>>>> index a37a2d5..e6e9334 100644
> >>>>>>>> --- a/include/hw/pci/pci.h
> >>>>>>>> +++ b/include/hw/pci/pci.h
> >>>>>>>> @@ -283,6 +283,12 @@ struct PCIDevice {
> >>>>>>>>       * MSI). For conventional PCI root complex, this field is
> >>>>>>>>       * meaningless. */
> >>>>>>>>      PCIReqIDCache requester_id_cache;
> >>>>>>>> +    /* Some platforms need a unique ID for IOMMU source 
> >>>>>>>> identification
> >>>>>>>> +     * or MSI source identification. QEMU implements a simple 
> >>>>>>>> scheme:
> >>>>>>>> +     * stream_id =  stream_id_base + requester_id. The 
> >>>>>>>> stream_id_base will
> >>>>>>>> +     * ensure that all the devices in the system have different 
> >>>>>>>> stream ID
> >>>>>>>> +     * domains */
> >>>>>>>> +    uint32_t stream_id_base;
> >>>>>>> get rid of IOMMU terminology?
> >>>>>>>
> >>>>>>> Note that when adding other sub-systems you will need to address the
> >>>>>>> ACPI side as the IORT table built by hw/arm/virt-acpi-build.c 
> >>>>>>> currently
> >>>>>>> defines an RID mapping for the single root complex.
> >>>>>>>
> >>>>>>> Thanks
> >>>>>>>
> >>>>>>> Eric
> >>>>>>>>      char name[64];
> >>>>>>>>      PCIIORegion io_regions[PCI_NUM_REGIONS];
> >>>>>>>>      AddressSpace bus_master_as;
> >>>>>>>> @@ -737,6 +743,8 @@ static inline uint16_t pci_get_bdf(PCIDevice 
> >>>>>>>> *dev)
> >>>>>>>>  
> >>>>>>>>  uint16_t pci_requester_id(PCIDevice *dev);
> >>>>>>>>  
> >>>>>>>> +uint32_t pci_stream_id(PCIDevice *dev);
> >>>>>>>> +
> >>>>>>>>  /* DMA access functions */
> >>>>>>>>  static inline AddressSpace *pci_get_address_space(PCIDevice *dev)
> >>>>>>>>  {
> >>>>>>>> diff --git a/kvm-all.c b/kvm-all.c
> >>>>>>>> index 90b8573..5a508c3 100644
> >>>>>>>> --- a/kvm-all.c
> >>>>>>>> +++ b/kvm-all.c
> >>>>>>>> @@ -1280,7 +1280,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int 
> >>>>>>>> vector, PCIDevice *dev)
> >>>>>>>>      kroute.u.msi.data = le32_to_cpu(msg.data);
> >>>>>>>>      if (kvm_msi_devid_required()) {
> >>>>>>>>          kroute.flags = KVM_MSI_VALID_DEVID;
> >>>>>>>> -        kroute.u.msi.devid = pci_requester_id(dev);
> >>>>>>>> +        kroute.u.msi.devid = pci_stream_id(dev);
> >>>>>>>>      }
> >>>>>>>>      if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data, 
> >>>>>>>> dev)) {
> >>>>>>>>          kvm_irqchip_release_virq(s, virq);
> >>>>>>>> @@ -1317,7 +1317,7 @@ int kvm_irqchip_update_msi_route(KVMState *s, 
> >>>>>>>> int virq, MSIMessage msg,
> >>>>>>>>      kroute.u.msi.data = le32_to_cpu(msg.data);
> >>>>>>>>      if (kvm_msi_devid_required()) {
> >>>>>>>>          kroute.flags = KVM_MSI_VALID_DEVID;
> >>>>>>>> -        kroute.u.msi.devid = pci_requester_id(dev);
> >>>>>>>> +        kroute.u.msi.devid = pci_stream_id(dev);
> >>>>>>>>      }
> >>>>>>>>      if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data, 
> >>>>>>>> dev)) {
> >>>>>>>>          return -EINVAL;
> >>>>>>>>
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]