From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:50569)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <bharata.rao@gmail.com>) id 1b18md-0000S9-Og
	for qemu-devel@nongnu.org; Fri, 13 May 2016 04:49:20 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <bharata.rao@gmail.com>) id 1b18mb-0005rF-5h
	for qemu-devel@nongnu.org; Fri, 13 May 2016 04:49:18 -0400
MIME-Version: 1.0
In-Reply-To: <CAGZKiBqVGYEqU3h1j7NjHeFXnMriKAd5G0ijrEOAW6BCMfHE9Q@mail.gmail.com>
References: <1462344751-28281-1-git-send-email-aik@ozlabs.ru>
	<1462344751-28281-20-git-send-email-aik@ozlabs.ru>
	<CAGZKiBqVGYEqU3h1j7NjHeFXnMriKAd5G0ijrEOAW6BCMfHE9Q@mail.gmail.com>
Date: Fri, 13 May 2016 14:19:12 +0530
Message-ID: <CAGZKiBqOO2ze8utfSB2wvrMmNPD=4ewhDNqeg93RFARPOXf9Tw@mail.gmail.com>
From: Bharata B Rao <bharata.rao@gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH qemu v16 19/19] spapr_pci/spapr_pci_vfio:
 Support Dynamic DMA Windows (DDW)
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, Alexander Graf <agraf@suse.de>, Alex Williamson <alex.williamson@redhat.com>, "qemu-ppc@nongnu.org" <qemu-ppc@nongnu.org>, Paolo Bonzini <pbonzini@redhat.com>, David Gibson <david@gibson.dropbear.id.au>

On Fri, May 13, 2016 at 2:11 PM, Bharata B Rao <bharata.rao@gmail.com> wrote:
> On Wed, May 4, 2016 at 12:22 PM, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>> +static void rtas_ibm_create_pe_dma_window(PowerPCCPU *cpu,
>> +                                          sPAPRMachineState *spapr,
>> +                                          uint32_t token, uint32_t nargs,
>> +                                          target_ulong args,
>> +                                          uint32_t nret, target_ulong rets)
>> +{
>> +    sPAPRPHBState *sphb;
>> +    sPAPRTCETable *tcet = NULL;
>> +    uint32_t addr, page_shift, window_shift, liobn;
>> +    uint64_t buid;
>> +
>> +    if ((nargs != 5) || (nret != 4)) {
>> +        goto param_error_exit;
>> +    }
>> +
>> +    buid = ((uint64_t)rtas_ld(args, 1) << 32) | rtas_ld(args, 2);
>> +    addr = rtas_ld(args, 0);
>> +    sphb = spapr_pci_find_phb(spapr, buid);
>> +    if (!sphb || !sphb->ddw_enabled) {
>> +        goto param_error_exit;
>> +    }
>> +
>> +    page_shift = rtas_ld(args, 3);
>> +    window_shift = rtas_ld(args, 4);
>
> Kernel has a bug due to which wrong window_shift gets returned here. I
> have posted possible fix here:
> https://patchwork.ozlabs.org/patch/621497/
>
> I have tried to work around this issue in QEMU too
> https://lists.nongnu.org/archive/html/qemu-ppc/2016-04/msg00226.html
>
> But the above work around involves changing the memory representation
> in DT. Hence I feel until the guest kernel changes are available, a
> simpler work around would be to discard the window_shift value above
> and recalculate the right value as below:
>
> if (machine->ram_size == machine->maxram_size) {
>     max_window_size = machine->ram_size;
> } else {
>      MemoryHotplugState *hpms = &spapr->hotplug_memory;
>      max_window_size = hpms->base + memory_region_size(&hpms->mr);
> }
> window_shift = max_window_size >> SPAPR_TCE_PAGE_SHIFT;
>
> and create DDW based on this calculated window_shift value. Does that
> sound reasonable ?

Sorry, missed mentioning earlier that incorrect DDW value here causes
memory hotplug to fail.

>
> Regards,
> Bharata.



-- 
http://raobharata.wordpress.com/