From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59562) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRpFM-0000ax-Av for qemu-devel@nongnu.org; Wed, 10 Sep 2014 17:16:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XRpFF-0007nD-Fr for qemu-devel@nongnu.org; Wed, 10 Sep 2014 17:16:12 -0400 Message-ID: <5410BF90.4090609@suse.de> Date: Wed, 10 Sep 2014 23:16:00 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1409307142-2600-1-git-send-email-aik@ozlabs.ru> <1409307142-2600-14-git-send-email-aik@ozlabs.ru> <54104BBA.1010207@suse.de> <54106715.5010401@ozlabs.ru> In-Reply-To: <54106715.5010401@ozlabs.ru> Content-Type: text/plain; charset=koi8-r Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v3 13/18] spapr_pci: Enable DDW List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: Alex Williamson , qemu-ppc@nongnu.org, David Gibson 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 >>> --- >>> 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