From mboxrd@z Thu Jan  1 00:00:00 1970
From: Peter Xu <peterx@redhat.com>
Subject: Re: [PATCH v7 04/14] x86: intel-iommu: add vt-d init test
Date: Mon, 5 Dec 2016 10:31:35 +0800
Message-ID: <20161205023135.GI21601@pxdev.xzpeter.org>
References: <1480393550-12385-1-git-send-email-peterx@redhat.com>
 <1480393550-12385-5-git-send-email-peterx@redhat.com>
 <20161201131452.GC13263@agordeev.lab.eng.brq.redhat.com>
 <20161202031234.GB21601@pxdev.xzpeter.org>
 <20161202075017.GF323@agordeev.lab.eng.brq.redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Cc: kvm@vger.kernel.org, drjones@redhat.com, jan.kiszka@web.de,
        rkrcmar@redhat.com, pbonzini@redhat.com
To: Alexander Gordeev <agordeev@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:45068 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751433AbcLECbs (ORCPT <rfc822;kvm@vger.kernel.org>);
        Sun, 4 Dec 2016 21:31:48 -0500
Content-Disposition: inline
In-Reply-To: <20161202075017.GF323@agordeev.lab.eng.brq.redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Fri, Dec 02, 2016 at 08:50:17AM +0100, Alexander Gordeev wrote:

[...]

> > > > +	vtd_dump_init_info();
> > > 
> > > Should we check an iommu is there indeed? My environment
> > > returns all zeroes (which is wrong I guess) and attempts
> > > to proceed.
> > 
> > How about an assertion on the version? We just let it quit with error
> > if with a wrong version (in this case, version is all zeros).
> 
> Yep. I guess (!max && max >= min) should fit?

>>From vt-d spec 10.4.1, "MAX" is "Major Version number", and "MIN" is
"Minor Version number". So looks like it is possible we have max=1 and
min>1 - minor version increases, while major version keeps. To make it
simpler, I'll use (max >= 1) directly.

> 
> > [...]
> > 
> > > > +static inline uint64_t vtd_readq(unsigned int reg)
> > > > +{
> > > > +	return __raw_readq(vtd_reg(reg));
> > > > +}
> > > 
> > > The accessors above should use ioremap'ped pointer, not direct
> > > access to Q35_HOST_BRIDGE_IOMMU_ADDR.
> > 
> > Hmm... This issue applies to EDU device register accesses as well.
> > Will fix.
> > 
> > Here I think I can use phys_to_virt() directly since x86_64 has
> > already mapped the first 4G memory as 1:1. However for EDU register
> > access I'd better use ioremap() since it might be used outside x86_64
> > in the future.
> 
> Actually, I think for x86_64 you need to ioremap as well. Because (a)
> accessing MMIO with no prior call to ioremap() is rather confusing
> and (b) we never know what machine this code might run in, let's say
> 10 years ;)

Ok. Let me fix. Thanks!

-- peterx