Re: [Qemu-devel] [RFC PATCH 07/10] spapr_pci: Allow PCI host bridge DMA window to be configured

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 09/24/2015 04:59 PM, Laurent Vivier wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
>
> On 24/09/2015 01:54, David Gibson wrote:
>> On Wed, Sep 23, 2015 at 08:55:01PM +0200, Laurent Vivier wrote:
>>>
>>>
>>> On 17/09/2015 15:09, David Gibson wrote:
>>>> At present the PCI host bridge (PHB) for the pseries machine
>>>> type has a fixed DMA window from 0..1GB (in PCI address space)
>>>> which is mapped to real memory via the PAPR paravirtualized
>>>> IOMMU.
>>>>
>>>> For better support of VFIO devices, we're going to want to
>>>> allow for different configurations of the DMA window.
>>>>
>>>> Eventually we'll want to allow the guest itself to reconfigure
>>>> the window via the PAPR dynamic DMA window interface, but as a
>>>> preliminary this patch allows the user to reconfigure the
>>>> window with new properties on the PHB device.
>>>>
>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au> ---
>>>> hw/ppc/spapr_pci.c          | 7 +++++--
>>>> include/hw/pci-host/spapr.h | 3 +-- 2 files changed, 6
>>>> insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index
>>>> b088491..622c4ac 100644 --- a/hw/ppc/spapr_pci.c +++
>>>> b/hw/ppc/spapr_pci.c @@ -1394,7 +1394,7 @@ 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; +
>>>> nb_table = sphb->dma_win_size >> SPAPR_TCE_PAGE_SHIFT; tcet =
>>>> spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn, 0,
>>>> SPAPR_TCE_PAGE_SHIFT, nb_table, false); if (!tcet) { @@ -1404,7
>>>> +1404,7 @@ static void spapr_phb_finish_realize(sPAPRPHBState
>>>> *sphb, Error **errp) }
>>>>
>>>> /* Register default 32bit DMA window */ -
>>>> memory_region_add_subregion(&sphb->iommu_root, 0, +
>>>> memory_region_add_subregion(&sphb->iommu_root,
>>>> sphb->dma_win_addr, spapr_tce_get_iommu(tcet)); }
>>>>
>>>> @@ -1437,6 +1437,9 @@ static Property spapr_phb_properties[] =
>>>> { SPAPR_PCI_IO_WIN_SIZE),
>>>> DEFINE_PROP_BOOL("dynamic-reconfiguration", sPAPRPHBState,
>>>> dr_enabled, true), +    /* Default DMA window is 0..1GB */ +
>>>> DEFINE_PROP_UINT64("dma_win_addr", sPAPRPHBState, dma_win_addr,
>>>> 0), +    DEFINE_PROP_UINT64("dma_win_size", sPAPRPHBState,
>>>> dma_win_size, 0x40000000), DEFINE_PROP_END_OF_LIST(), };
>>>
>>> Add them too to the vmstate_spapr_pci ?
>>
>> No.  At least, not just now.
>>
>> This is the sort of configuration information that typically isn't
>> migrated, instead requiring the far end to be set up matching.
>> Which
>
> I think this is the aim of VMSTATE_UINT64_EQUAL() ?

We use it only for things which cannot be set via the command line and 
ideally there should be no VMSTATE_*_EQUAL. If something can be set via the 
command line, then the management software (read - libvirt) runs QEMU with 
explicit parameters to guarantee that these are equal. Checking it in 
VMSTATE is too late when migrating with full disk copy - it takes a lot of 
time before the destination QEMU discovers that it cannot actually run.


>
>> is a problem, but not one within the scope of this patch series.
>> In any case just transferring the values wouldn't be enough - we'd
>> need post_load handlers to actually rewire the TCE table
>> accordingly.
>>
>> We will need to do this once we add dynamic DMA window support,
>> but again, not in scope just now.
>>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
>
> iEYEARECAAYFAlYDn1UACgkQNKT2yavzbFM20ACfcUVIPLObcYh4y5U8CcoLVGoO
> FR8AoOgigTMFu7FOh7wI8U+fGNtYv6Ji
> =4opa
> -----END PGP SIGNATURE-----
>


-- 
Alexey