Re: [Qemu-devel] [RFC PATCH v3 13/18] spapr_pci: Enable DDW

[Alexander Graf <agraf@suse.de>]

On 10.09.14 16:58, Alexey Kardashevskiy wrote:
> On 09/10/2014 11:01 PM, Alexander Graf wrote:
>>
>>
>> On 29.08.14 12:12, Alexey Kardashevskiy wrote:
>>> This implements DDW for emulated PHB.
>>>
>>> This advertises the query/create/remove RTAS tokens in device tree.
>>> This does not advertise the reset RTAS token though, will be added later.
>>>
>>> The "ddw" property is enabled by default on a PHB but for compatibility
>>> pseries-2.1 machine disables it.
>>>
>>> Since QEMU does not implement any 64bit DMA capable device, this hack
>>> has been used to enable 64bit DMA on E1000:
>>>
>>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>>> index 0fc29a0..131f80a 100644
>>> --- a/hw/net/e1000.c
>>> +++ b/hw/net/e1000.c
>>> @@ -240,6 +240,7 @@ static const uint32_t mac_reg_init[] = {
>>>      [STATUS] =  0x80000000 | E1000_STATUS_GIO_MASTER_ENABLE |
>>>                  E1000_STATUS_ASDV | E1000_STATUS_MTXCKOK |
>>>                  E1000_STATUS_SPEED_1000 | E1000_STATUS_FD |
>>> +                E1000_STATUS_PCIX_MODE |
>>>                  E1000_STATUS_LU,
>>>      [MANC] =    E1000_MANC_EN_MNG2HOST | E1000_MANC_RCV_TCO_EN |
>>>                  E1000_MANC_ARP_EN | E1000_MANC_0298_EN |
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> Changes:
>>> v3:
>>> * removed reset
>>> * windows_num is now 1 or bigger rather than 0-based value and it is only
>>> changed in PHB code, not in RTAS
>>> * added page mask check in create()
>>>
>>> v2:
>>> * tested on hacked emulated E1000
>>> * implemented DDW reset on the PHB reset
>>> * spapr_pci_ddw_remove/spapr_pci_ddw_reset are public for reuse by VFIO
>>> ---
>>>  hw/ppc/spapr.c              |  9 +++++
>>>  hw/ppc/spapr_pci.c          | 94 +++++++++++++++++++++++++++++++++++++++++++++
>>>  include/hw/pci-host/spapr.h |  7 ++++
>>>  3 files changed, 110 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index d2d3c27..663cb75 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1670,11 +1670,20 @@ static const TypeInfo spapr_machine_info = {
>>>  static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data)
>>>  {
>>>      MachineClass *mc = MACHINE_CLASS(oc);
>>> +    static GlobalProperty compat_props[] = {
>>> +        {
>>> +            .driver   = TYPE_SPAPR_PCI_HOST_BRIDGE,
>>> +            .property = "ddw",
>>> +            .value    = stringify(off),
>>> +        },
>>> +        { /* end of list */ }
>>> +    };
>>>  
>>>      mc->name = "pseries-2.1";
>>>      mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1";
>>>      mc->alias = "pseries";
>>>      mc->is_default = 1;
>>> +    mc->compat_props = compat_props;
>>>  }
>>>  
>>>  static const TypeInfo spapr_machine_2_1_info = {
>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>> index 2968b39..04ee1dc 100644
>>> --- a/hw/ppc/spapr_pci.c
>>> +++ b/hw/ppc/spapr_pci.c
>>> @@ -470,6 +470,76 @@ static const MemoryRegionOps spapr_msi_ops = {
>>>  };
>>>  
>>>  /*
>>> + * Dynamic DMA windows
>>> + */
>>> +static int spapr_pci_ddw_query(sPAPRPHBState *sphb,
>>> +                               uint32_t *windows_available,
>>> +                               uint32_t *page_size_mask)
>>> +{
>>> +    *windows_available = 1;
>>> +    *page_size_mask = DDW_PGSIZE_64K | DDW_PGSIZE_16M;
>>> +
>>> +    return 0;
>>
>> What exactly does this return? The number of still available windows?
>> The total number of windows?
> 
> 1. Number of available windows
> 2. page masks supported for new windows.
> 
> This is what RTAS's counterpart does. We can change it to something better
> if we want but I cannot think of anything better.

The reason I'm asking is that I don't quite grasp why we're always
returning 1 available window.

1 window is already taken by the default window, no? So "1" naturally
would mean no ddw window. If we ignore that one, what if I create 1 ddw
window? The call would still return 1, so could I create another one? If
I can, why didn't the call return 2 in the first place?


Alex