Re: [PATCH 4/4] iommu/x86: make unity range checking more strict

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: Paul Durrant <paul@xen.org>, xen-devel@lists.xenproject.org
Subject: Re: [PATCH 4/4] iommu/x86: make unity range checking more strict
Date: Wed, 7 Feb 2024 10:14:52 +0100	[thread overview]
Message-ID: <ZcNKDKD4G7fpIX80@macbook> (raw)
In-Reply-To: <5db22ee1-40b3-4df8-88b8-446a2e224d22@suse.com>

On Tue, Feb 06, 2024 at 12:49:08PM +0100, Jan Beulich wrote:
> On 01.02.2024 18:01, Roger Pau Monne wrote:
> > Currently when a unity range overlaps with memory being used as RAM by the
> > hypervisor the result would be that the IOMMU gets disabled.  However that's
> > not enough, as even with the IOMMU disabled the device will still access the
> > affected RAM areas.
> 
> Hmm, no, I think this is going too far. Not the least because it is
> s/will/may/. But also because if we really wanted such behavior, we
> ought to also parse the respective ACPI tables when the "iommu=off".

I guessed so, hence why it's the last patch in the series.  TBH I
think it's very unlikely that such system exist.

> > Note that IVMD or RMRR ranges being placed over RAM is a firmware bug.
> 
> As written this is wrong: They're typically in RAM, just that the E820
> type for that range should not be RAM_TYPE_CONVENTIONAL.

Hm, yes, ÇI should have written 'over a RAM range in the memory map'
or similar.

> > Doing so also allows to simplify the code and use a switch over the reported
> > memory type(s).
> 
> I'm afraid this isn't right either: page_get_ram_type() can set
> multiple bits in its output.

It can indeed.  But if the only bit set is RAM_TYPE_CONVENTIONAL then
the page will be handled as RAM, and that's where Xen would be in
trouble if a device is also using such page as a unity map.

If the page however is RAM_TYPE_CONVENTIONAL | RAM_TYPE_RESERVED then
the RESERVED type will take over the whole page, and it's no longer an
issue to have a unity range covering it.

Thanks, Roger.