[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 05/21] IOMMU/x86: restrict IO-APIC mappings for PV Dom0


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 4 May 2022 14:12:58 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=PA5UUpOtBj8BJ1Me5KMvvF/KdLRLs9SOHPLbpRHMCwY=; b=kFdHFOfYuAIBcb8fEJgPwWtzUY0EPEBZjDlFPkiG9f9MtT+fj86w3D6/DISgXRiVdCl+bnDLxlmnyafoZla5PntzY/5OZ6a1XMTCc5N4BQEkDsNVcwHVfi27Mi1bk/xKcNFFLVkaAa/0J+/nahgWfdhVL5mUDFw3ANxQZFP7TVvI1zHDJsB0o+MPkx7zHWnuHKZwVvZjRKSZLT1sKGMAPU9NGckDv8iPcYM1nwJ0POTnxrzEDpJT8OdP8h8vcAQNmRV2xrxa9ksavBFAmYs2K6sn3EpeLJJC/rKYEL7Ti8FMvcAcivk4u6DdgN/D4/fLSB7Z7KgXlDZZMCB0ACQVqA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RrDcdguwB1tFuk1fYTsmlhU3RGvhs+WHzDfKwqyFoM/+oRB596JoUJZLJtom5fN2xo/MsSoUv/5cKYP1Qqp00qLdwaZSOwJFHb3cHwKN0YyftdaGRKPjq4XY4FjDjaEOjWxGJCyEtk3ylJFjROV1br6RpKU94N6w+wtMMFDHgXfDFVHyt3Uzf71QHHdOvmYLxmSaHAgD+9JzKJM0WQbKjCTOihcky2SpklSm4XjXSMfsC9vuZ/E+lAnBDVh1OjGSM1fEoG4P3cPqlVB3ZpJZnoMmPFFIIMJT6sZDT2yshb/lpeTSQr8bRhcRDDw4pqFdLifylmJp3492dxqaEtKs0Q==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Wed, 04 May 2022 12:13:12 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 04.05.2022 14:01, Roger Pau Monné wrote:
> On Wed, May 04, 2022 at 12:51:25PM +0200, Jan Beulich wrote:
>> On 04.05.2022 12:30, Roger Pau Monné wrote:
>>> Right, ->iomem_caps is indeed too wide for our purpose.  What
>>> about using something like:
>>>
>>> else if ( is_pv_domain(d) )
>>> {
>>>     if ( !iomem_access_permitted(d, pfn, pfn) )
>>>         return 0;
>>
>> We can't return 0 here (as RAM pages also make it here when
>> !iommu_hwdom_strict), so I can at best take this as a vague outline
>> of what you really mean. And I don't want to rely on RAM pages being
>> (imo wrongly) represented by set bits in Dom0's iomem_caps.
> 
> Well, yes, my suggestion was taking into account that ->iomem_caps for
> dom0 has mostly holes for things that shouldn't be mapped, but
> otherwise contains everything else as allowed (including RAM).
> 
> We could instead do:
> 
> else if ( is_pv_domain(d) && type != RAM_TYPE_CONVENTIONAL )
> {
>     ...
> 
> So that we don't rely on RAM being 'allowed' in ->iomem_caps?

This would feel to me like excess special casing.

>>>     if ( rangeset_contains_singleton(mmio_ro_ranges, pfn) )
>>>         return IOMMUF_readable;
>>> }
>>>
>>> That would get us a bit closer to allowed CPU side mappings, and we
>>> don't need to special case IO-APIC or HPET addresses as those are
>>> already added to ->iomem_caps or mmio_ro_ranges respectively by
>>> dom0_setup_permissions().
>>
>> This won't fit in a region of code framed by a (split) comment
>> saying "Check that it doesn't overlap with ...". Hence if anything
>> I could put something like this further down. Yet even then the
>> question remains what to do with ranges which pass
>> iomem_access_permitted() but
>> - aren't really MMIO,
>> - are inside MMCFG,
>> - are otherwise special.
>>
>> Or did you perhaps mean to suggest something like
>>
>> else if ( is_pv_domain(d) && iomem_access_permitted(d, pfn, pfn) &&
>>           rangeset_contains_singleton(mmio_ro_ranges, pfn) )
>>     return IOMMUF_readable;
> 
> I don't think this would be fully correct, as we would still allow
> mappings of IO-APIC pages explicitly banned in ->iomem_caps by not
> handling those?

CPU side mappings don't deal with the IO-APICs specifically. They only
care about iomem_caps and mmio_ro_ranges. Hence explicitly banned
IO-APIC pages cannot be mapped there either. (Of course we only do
such banning if IO-APIC pages weren't possible to represent in
mmio_ro_ranges, which should effectively be never.)

>> ? Then there would only remain the question of whether mapping r/o
>> MMCFG pages is okay (I don't think it is), but that could then be
>> special-cased similar to what's done further down for vPCI (by not
>> returning in the "else if", but merely updating "perms").
> 
> Well part of the point of this is to make CPU and Device mappings
> more similar.  I don't think devices have any business in poking at
> the MMCFG range, so it's fine to explicitly ban that range.  But I
> would have also said the same for IO-APIC pages, so I'm unsure why are
> IO-APIC pages fine to be mapped RO, but not the MMCFG range.

I wouldn't have wanted to allow r/o mappings of the IO-APICs, but
Linux plus the ACPI tables of certain vendors require us to permit
this. If we didn't, Dom0 would crash there during boot.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.