[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH qemu v10 03/14] spapr_pci: Convert fi
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH qemu v10 03/14] spapr_pci: Convert finish_realize() to dma_capabilities_update()+dma_init_window() |
Date: |
Mon, 06 Jul 2015 18:41:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 |
On 06/07/2015 04:10, Alexey Kardashevskiy wrote:
> This reworks finish_realize() which used to finalize DMA setup with
> an assumption that it will not change later.
>
> New callbacks supports various window parameters such as page and
> windows sizes. The new callback return error code rather than Error**.
>
> This is a mechanical change so no change in behaviour is expected.
> This is a part of getting rid of spapr-pci-vfio-host-bridge type.
>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> Reviewed-by: David Gibson <address@hidden>
> ---
> Changes:
> v8:
> * moved spapr_phb_dma_capabilities_update() higher to avoid forward
> declaration in following patches and keep DMA code together (i.e. next
> to spapr_pci_dma_iommu())
> ---
> hw/ppc/spapr_pci.c | 59
> ++++++++++++++++++++++++++-------------------
> hw/ppc/spapr_pci_vfio.c | 53 ++++++++++++++++------------------------
> include/hw/pci-host/spapr.h | 8 +++++-
> 3 files changed, 62 insertions(+), 58 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index a8f79d8..c1ca13d 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -808,6 +808,28 @@ static char *spapr_phb_get_loc_code(sPAPRPHBState *sphb,
> PCIDevice *pdev)
> return buf;
> }
>
> +static int spapr_phb_dma_capabilities_update(sPAPRPHBState *sphb)
> +{
> + sphb->dma32_window_start = 0;
> + sphb->dma32_window_size = SPAPR_PCI_DMA32_SIZE;
> +
> + return 0;
> +}
> +
> +static int spapr_phb_dma_init_window(sPAPRPHBState *sphb,
> + uint32_t liobn, uint32_t page_shift,
> + uint64_t window_size)
> +{
> + uint64_t bus_offset = sphb->dma32_window_start;
> + sPAPRTCETable *tcet;
> +
> + tcet = spapr_tce_new_table(DEVICE(sphb), liobn, bus_offset, page_shift,
> + window_size >> page_shift,
> + false);
> +
> + return tcet ? 0 : -1;
> +}
> +
> /* Macros to operate with address in OF binding to PCI */
> #define b_x(x, p, l) (((x) & ((1<<(l))-1)) << (p))
> #define b_n(x) b_x((x), 31, 1) /* 0 if relocatable */
> @@ -1220,6 +1242,7 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> int i;
> PCIBus *bus;
> uint64_t msi_window_size = 4096;
> + sPAPRTCETable *tcet;
>
> if (sphb->index != (uint32_t)-1) {
> hwaddr windows_base;
> @@ -1369,33 +1392,18 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> }
> }
>
> - if (!info->finish_realize) {
> - error_setg(errp, "finish_realize not defined");
> - return;
> - }
> -
> - info->finish_realize(sphb, errp);
> -
> - sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free,
> g_free);
> -}
> -
> -static void spapr_phb_finish_realize(sPAPRPHBState *sphb, Error **errp)
> -{
> - sPAPRTCETable *tcet;
> - uint32_t nb_table;
> -
> - nb_table = SPAPR_PCI_DMA32_SIZE >> SPAPR_TCE_PAGE_SHIFT;
> - tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
> - 0, SPAPR_TCE_PAGE_SHIFT, nb_table, false);
> + info->dma_capabilities_update(sphb);
> + info->dma_init_window(sphb, sphb->dma_liobn, SPAPR_TCE_PAGE_SHIFT,
> + sphb->dma32_window_size);
> + tcet = spapr_tce_find_by_liobn(sphb->dma_liobn);
> if (!tcet) {
> - error_setg(errp, "Unable to create TCE table for %s",
> - sphb->dtbusname);
> - return ;
> + error_setg(errp, "failed to create TCE table");
> + return;
> }
> -
> - /* Register default 32bit DMA window */
> - memory_region_add_subregion(&sphb->iommu_root, 0,
> + memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset,
> spapr_tce_get_iommu(tcet));
> +
> + sphb->msi = g_hash_table_new_full(g_int_hash, g_int_equal, g_free,
> g_free);
> }
>
> static int spapr_phb_children_reset(Object *child, void *opaque)
> @@ -1543,9 +1551,10 @@ static void spapr_phb_class_init(ObjectClass *klass,
> void *data)
> dc->vmsd = &vmstate_spapr_pci;
> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> dc->cannot_instantiate_with_device_add_yet = false;
> - spc->finish_realize = spapr_phb_finish_realize;
> hp->plug = spapr_phb_hot_plug_child;
> hp->unplug = spapr_phb_hot_unplug_child;
> + spc->dma_capabilities_update = spapr_phb_dma_capabilities_update;
> + spc->dma_init_window = spapr_phb_dma_init_window;
> }
>
> static const TypeInfo spapr_phb_info = {
> diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
> index cca45ed..6e3e17b 100644
> --- a/hw/ppc/spapr_pci_vfio.c
> +++ b/hw/ppc/spapr_pci_vfio.c
> @@ -28,48 +28,36 @@ static Property spapr_phb_vfio_properties[] = {
> DEFINE_PROP_END_OF_LIST(),
> };
>
> -static void spapr_phb_vfio_finish_realize(sPAPRPHBState *sphb, Error **errp)
> +static int spapr_phb_vfio_dma_capabilities_update(sPAPRPHBState *sphb)
> {
> sPAPRPHBVFIOState *svphb = SPAPR_PCI_VFIO_HOST_BRIDGE(sphb);
> struct vfio_iommu_spapr_tce_info info = { .argsz = sizeof(info) };
> int ret;
> - sPAPRTCETable *tcet;
> - uint32_t liobn = svphb->phb.dma_liobn;
>
> - if (svphb->iommugroupid == -1) {
> - error_setg(errp, "Wrong IOMMU group ID %d", svphb->iommugroupid);
> - return;
> - }
> -
> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> - VFIO_CHECK_EXTENSION,
> - (void *) VFIO_SPAPR_TCE_IOMMU);
This ioctl() disappears completely, was it useless ?
[ but I guess the following one will fail exactly in the same way ]
> - if (ret != 1) {
> - error_setg_errno(errp, -ret,
> - "spapr-vfio: SPAPR extension is not supported");
> - return;
> - }
> -
> - ret = vfio_container_ioctl(&svphb->phb.iommu_as, svphb->iommugroupid,
> + ret = vfio_container_ioctl(&sphb->iommu_as, svphb->iommugroupid,
> VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
> if (ret) {
> - error_setg_errno(errp, -ret,
> - "spapr-vfio: get info from container failed");
> - return;
> + return ret;
> }
>
> - tcet = spapr_tce_new_table(DEVICE(sphb), liobn, info.dma32_window_start,
> - SPAPR_TCE_PAGE_SHIFT,
> - info.dma32_window_size >>
> SPAPR_TCE_PAGE_SHIFT,
> + sphb->dma32_window_start = info.dma32_window_start;
> + sphb->dma32_window_size = info.dma32_window_size;
> +
> + return ret;
> +}
> +
> +static int spapr_phb_vfio_dma_init_window(sPAPRPHBState *sphb,
> + uint32_t liobn, uint32_t
> page_shift,
> + uint64_t window_size)
> +{
> + uint64_t bus_offset = sphb->dma32_window_start;
> + sPAPRTCETable *tcet;
> +
> + tcet = spapr_tce_new_table(DEVICE(sphb), liobn, bus_offset, page_shift,
> + window_size >> page_shift,
> true);
> - if (!tcet) {
> - error_setg(errp, "spapr-vfio: failed to create VFIO TCE table");
> - return;
> - }
>
> - /* Register default 32bit DMA window */
> - memory_region_add_subregion(&sphb->iommu_root, tcet->bus_offset,
> - spapr_tce_get_iommu(tcet));
> + return tcet ? 0 : -1;
> }
>
> static void spapr_phb_vfio_eeh_reenable(sPAPRPHBVFIOState *svphb)
> @@ -257,7 +245,8 @@ static void spapr_phb_vfio_class_init(ObjectClass *klass,
> void *data)
>
> dc->props = spapr_phb_vfio_properties;
> dc->reset = spapr_phb_vfio_reset;
> - spc->finish_realize = spapr_phb_vfio_finish_realize;
> + spc->dma_capabilities_update = spapr_phb_vfio_dma_capabilities_update;
> + spc->dma_init_window = spapr_phb_vfio_dma_init_window;
> spc->eeh_set_option = spapr_phb_vfio_eeh_set_option;
> spc->eeh_get_state = spapr_phb_vfio_eeh_get_state;
> spc->eeh_reset = spapr_phb_vfio_eeh_reset;
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 5322b56..b6d5719 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -48,7 +48,10 @@ typedef struct sPAPRPHBVFIOState sPAPRPHBVFIOState;
> struct sPAPRPHBClass {
> PCIHostBridgeClass parent_class;
>
> - void (*finish_realize)(sPAPRPHBState *sphb, Error **errp);
> + int (*dma_capabilities_update)(sPAPRPHBState *sphb);
> + int (*dma_init_window)(sPAPRPHBState *sphb,
> + uint32_t liobn, uint32_t page_shift,
> + uint64_t window_size);
> int (*eeh_set_option)(sPAPRPHBState *sphb, unsigned int addr, int
> option);
> int (*eeh_get_state)(sPAPRPHBState *sphb, int *state);
> int (*eeh_reset)(sPAPRPHBState *sphb, int option);
> @@ -90,6 +93,9 @@ struct sPAPRPHBState {
> int32_t msi_devs_num;
> spapr_pci_msi_mig *msi_devs;
>
> + uint32_t dma32_window_start;
> + uint32_t dma32_window_size;
> +
> QLIST_ENTRY(sPAPRPHBState) list;
> };
>
>
Really nice work.
Reviewed-by: Laurent Vivier <address@hidden>
- [Qemu-ppc] [PATCH qemu v10 00/14] spapr: vfio: Enable Dynamic DMA windows (DDW), Alexey Kardashevskiy, 2015/07/05
- [Qemu-ppc] [PATCH qemu v10 04/14] spapr_iommu: Move table allocation to helpers, Alexey Kardashevskiy, 2015/07/05
- [Qemu-ppc] [PATCH qemu v10 03/14] spapr_pci: Convert finish_realize() to dma_capabilities_update()+dma_init_window(), Alexey Kardashevskiy, 2015/07/05
- Re: [Qemu-ppc] [Qemu-devel] [PATCH qemu v10 03/14] spapr_pci: Convert finish_realize() to dma_capabilities_update()+dma_init_window(),
Laurent Vivier <=
- [Qemu-ppc] [PATCH qemu v10 02/14] vmstate: Define VARRAY with VMS_ALLOC, Alexey Kardashevskiy, 2015/07/05
- [Qemu-ppc] [PATCH qemu v10 01/14] linux-headers: Update to 4.2-rc1, Alexey Kardashevskiy, 2015/07/05
- [Qemu-ppc] [PATCH qemu v10 07/14] spapr_iommu: Add root memory region, Alexey Kardashevskiy, 2015/07/05
- [Qemu-ppc] [PATCH qemu v10 11/14] spapr_pci_vfio: Enable multiple groups per container, Alexey Kardashevskiy, 2015/07/05
- [Qemu-ppc] [PATCH qemu v10 08/14] spapr_pci: Do complete reset of DMA config when resetting PHB, Alexey Kardashevskiy, 2015/07/05