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

[Alexey Kardashevskiy]

On 09/11/2014 07:16 AM, Alexander Graf wrote:
> 
> 
> 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 <address@hidden>
>>>> ---
>>>> 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.


Oh. I see it now. Bug. Should 1 or 0 depending on whether big window was
created or not.

> 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?

"1" means 1 additional window can be created. It does not return "2"
because the default window is always there. When/If I enable "pe-dma-reset"
RTAS call, then the guest will be able to remove default DMA window and
then spapr_pci_ddw_query() will start returning "2" after the default
window is removed.


-- 
Alexey