From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peter Xu <peterx@redhat.com>
Subject: Re: [kvm-unit-tests PATCH v7 06/13] pci: Rework pci_bar_addr()
Date: Fri, 23 Sep 2016 15:14:04 +0800
Message-ID: <20160923071404.GB15411@pxdev.xzpeter.org>
References: <cover.1471434672.git.agordeev@redhat.com>
 <38dc0bd6c1350568f4978dba3373249753944a61.1471434672.git.agordeev@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Cc: kvm@vger.kernel.org, Thomas Huth <thuth@redhat.com>,
        Andrew Jones <drjones@redhat.com>
To: Alexander Gordeev <agordeev@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:41784 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1753233AbcIWHOJ (ORCPT <rfc822;kvm@vger.kernel.org>);
        Fri, 23 Sep 2016 03:14:09 -0400
Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
        (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
        (No client certificate requested)
        by mx1.redhat.com (Postfix) with ESMTPS id B38027CDE5
        for <kvm@vger.kernel.org>; Fri, 23 Sep 2016 07:14:08 +0000 (UTC)
Content-Disposition: inline
In-Reply-To: <38dc0bd6c1350568f4978dba3373249753944a61.1471434672.git.agordeev@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Hi, Alex,

I see that we are adding ARM tests in the series as well, so I am
thinking whether this work is a prepare work to test ARM SMMU?

Also, some comments below...

On Wed, Aug 17, 2016 at 02:07:07PM +0200, Alexander Gordeev wrote:

[...]

> -unsigned long pci_bar_addr(pcidevaddr_t dev, int bar_num)
> +phys_addr_t pci_bar_addr(pcidevaddr_t dev, int bar_num)
>  {
>  	uint32_t bar = pci_bar_get(dev, bar_num);
> +	uint32_t mask = pci_bar_mask(bar);
> +	uint64_t addr = bar & mask;
>  
> -	if (bar & PCI_BASE_ADDRESS_SPACE_IO)
> -		return bar & PCI_BASE_ADDRESS_IO_MASK;
> -	else
> -		return bar & PCI_BASE_ADDRESS_MEM_MASK;
> +	if (pci_bar_is64(dev, bar_num))
> +		addr |= (uint64_t)pci_bar_get(dev, bar_num + 1) << 32;
> +
> +	return pci_translate_addr(dev, addr);

Raw question: do we need to translate bar addresses as well?

[...]

> +static inline
> +phys_addr_t pci_translate_addr(pcidevaddr_t dev __unused, uint64_t addr)
> +{
> +    return addr;
> +}
> +

We are not using dev here (and in following patches), I don't know
ARM, but at least x86 will need this to translate IOVA into PA (in
other words, each device can have its own IO address space). If this
codes are for common, shall we consider that as well?

Thanks.

-- peterx