Re: [Qemu-devel] [PATCH qemu v18 5/5] spapr_pci/spapr_pci_vfio: Support Dynamic DMA Windows (DDW)

[Thomas Huth <thuth@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4214 bytes --]

On 22.06.2016 05:23, Alexey Kardashevskiy wrote:
> On 22/06/16 12:35, David Gibson wrote:
>> On Tue, Jun 21, 2016 at 11:14:05AM +1000, Alexey Kardashevskiy wrote:
>>> This adds support for Dynamic DMA Windows (DDW) option defined by
>>> the SPAPR specification which allows to have additional DMA window(s)
>>>
>>> The "ddw" property is enabled by default on a PHB but for compatibility
>>> the pseries-2.6 machine and older disable it.
>>> This also creates a single DMA window for the older machines to
>>> maintain backward migration.
>>>
>>> This implements DDW for PHB with emulated and VFIO devices. The host
>>> kernel support is required. The advertised IOMMU page sizes are 4K and
>>> 64K; 16M pages are supported but not advertised by default, in order to
>>> enable them, the user has to specify "pgsz" property for PHB and
>>> enable huge pages for RAM.
>>>
>>> The existing linux guests try creating one additional huge DMA window
>>> with 64K or 16MB pages and map the entire guest RAM to. If succeeded,
>>> the guest switches to dma_direct_ops and never calls TCE hypercalls
>>> (H_PUT_TCE,...) again. This enables VFIO devices to use the entire RAM
>>> and not waste time on map/unmap later. This adds a "dma64_win_addr"
>>> property which is a bus address for the 64bit window and by default
>>> set to 0x800.0000.0000.0000 as this is what the modern POWER8 hardware
>>> uses and this allows having emulated and VFIO devices on the same bus.
>>>
>>> This adds 4 RTAS handlers:
>>> * ibm,query-pe-dma-window
>>> * ibm,create-pe-dma-window
>>> * ibm,remove-pe-dma-window
>>> * ibm,reset-pe-dma-window
>>> These are registered from type_init() callback.
>>>
>>> These RTAS handlers are implemented in a separate file to avoid polluting
>>> spapr_iommu.c with PCI.
>>>
>>> This changes sPAPRPHBState::dma_liobn to an array to allow 2 LIOBNs.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
[...]
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index 9f28fb3..0cb51dd 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
[...]
>>> @@ -1515,7 +1531,8 @@ static void spapr_phb_reset(DeviceState *qdev)
>>>  static Property spapr_phb_properties[] = {
>>>      DEFINE_PROP_UINT32("index", sPAPRPHBState, index, -1),
>>>      DEFINE_PROP_UINT64("buid", sPAPRPHBState, buid, -1),
>>> -    DEFINE_PROP_UINT32("liobn", sPAPRPHBState, dma_liobn, -1),
>>> +    DEFINE_PROP_UINT32("liobn", sPAPRPHBState, dma_liobn[0], -1),
>>> +    DEFINE_PROP_UINT32("liobn64", sPAPRPHBState, dma_liobn[1], -1),
>>>      DEFINE_PROP_UINT64("mem_win_addr", sPAPRPHBState, mem_win_addr, -1),
>>>      DEFINE_PROP_UINT64("mem_win_size", sPAPRPHBState, mem_win_size,
>>>                         SPAPR_PCI_MMIO_WIN_SIZE),
>>> @@ -1527,6 +1544,11 @@ static Property spapr_phb_properties[] = {
>>>      /* 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_UINT64("dma64_win_addr", sPAPRPHBState, dma64_win_addr,
>>> +                       0x800000000000000ULL),
>>> +    DEFINE_PROP_BOOL("ddw", sPAPRPHBState, ddw_enabled, true),
>>> +    DEFINE_PROP_UINT64("pgsz", sPAPRPHBState, page_size_mask,
>>> +                       (1ULL << 12) | (1ULL << 16)),
>>>      DEFINE_PROP_END_OF_LIST(),
>>>  };
>>>  
>>> @@ -1603,7 +1625,7 @@ static const VMStateDescription vmstate_spapr_pci = {
>>>      .post_load = spapr_pci_post_load,
>>>      .fields = (VMStateField[]) {
>>>          VMSTATE_UINT64_EQUAL(buid, sPAPRPHBState),
>>> -        VMSTATE_UINT32_EQUAL(dma_liobn, sPAPRPHBState),
>>> +        VMSTATE_UNUSED(4), /* dma_liobn */
>>
>> It's not obvious to me why this change is necessary.
> 
> It is not. But I was touching liobn and this is a proper cleanup which
> needs to be done anyway as _EQUAL() macros are sort of deprecated and
> rather pointless.

Not sure, but if you mark this field as unused now, is migration
backwards to an older version of QEMU still working? If not, you might
need to bump the version number, too?

 Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]