| Message ID | 22e7036c-cf49-4160-bd26-fbba6b67ff5d@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | VT-d: SATC handling and ATS tidying | expand |
On Mon, Feb 05, 2024 at 02:57:30PM +0100, Jan Beulich wrote: > ..., thus allowing them to become static. There's nothing x86-specific > about these functions anyway. > > Since only the "iommu_inclusive_mapping" parameter declaration would be > left in the file, move that as well. There's nothing VT-d specific about > it (anymore?): "dom0-iommu=map-inclusive" is similarly generic, and > documentation also doesn't say anything. Hm, I guess documentation should at least say that iommu_inclusive_mapping is x86 specific, because it's not parsed on Arm and hence might give the wrong impression that it's actually acknowledged there. > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com> Albeit I think it would be better to put the parsing in generic iommu.c, so that the option gets parsed on Arm and arch_iommu_hwdom_init() can print a warning message about it not supported on Arm. Thanks, Roger.
On 09.02.2024 09:39, Roger Pau Monné wrote: > On Mon, Feb 05, 2024 at 02:57:30PM +0100, Jan Beulich wrote: >> ..., thus allowing them to become static. There's nothing x86-specific >> about these functions anyway. >> >> Since only the "iommu_inclusive_mapping" parameter declaration would be >> left in the file, move that as well. There's nothing VT-d specific about >> it (anymore?): "dom0-iommu=map-inclusive" is similarly generic, and >> documentation also doesn't say anything. > > Hm, I guess documentation should at least say that > iommu_inclusive_mapping is x86 specific, because it's not parsed on > Arm and hence might give the wrong impression that it's actually > acknowledged there. In v2 I'm adding "(x86)" there. >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Roger Pau Monné <roger.pau@citrix.com> Thanks. > Albeit I think it would be better to put the parsing in generic > iommu.c, so that the option gets parsed on Arm and > arch_iommu_hwdom_init() can print a warning message about it not > supported on Arm. Hmm, I would have considered doing things the other way around - make that part of parsing in parse_dom0_iommu_param() x86-only. I would feel odd to introduce an option to Arm, just to be able to report that it's unsupported. The more when generic option parsing code will already log unrecognized options (sadly such log messages aren't seen in the serial log, for being issued too early). But let's ask Arm folks what they'd prefer, by adding all of them to To:. Jan
On Mon, Feb 12, 2024 at 10:46:55AM +0100, Jan Beulich wrote: > On 09.02.2024 09:39, Roger Pau Monné wrote: > > On Mon, Feb 05, 2024 at 02:57:30PM +0100, Jan Beulich wrote: > >> ..., thus allowing them to become static. There's nothing x86-specific > >> about these functions anyway. > >> > >> Since only the "iommu_inclusive_mapping" parameter declaration would be > >> left in the file, move that as well. There's nothing VT-d specific about > >> it (anymore?): "dom0-iommu=map-inclusive" is similarly generic, and > >> documentation also doesn't say anything. > > > > Hm, I guess documentation should at least say that > > iommu_inclusive_mapping is x86 specific, because it's not parsed on > > Arm and hence might give the wrong impression that it's actually > > acknowledged there. > > In v2 I'm adding "(x86)" there. Maybe part of this should be done as a separate patch so it can be backported to stable branches? > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > > > Acked-by: Roger Pau Monné <roger.pau@citrix.com> > > Thanks. > > > Albeit I think it would be better to put the parsing in generic > > iommu.c, so that the option gets parsed on Arm and > > arch_iommu_hwdom_init() can print a warning message about it not > > supported on Arm. > > Hmm, I would have considered doing things the other way around - make > that part of parsing in parse_dom0_iommu_param() x86-only. I would > feel odd to introduce an option to Arm, just to be able to report > that it's unsupported. The more when generic option parsing code will > already log unrecognized options (sadly such log messages aren't seen > in the serial log, for being issued too early). But let's ask Arm > folks what they'd prefer, by adding all of them to To:. FWIW, that (moving part of the parsing of parse_dom0_iommu_param() to x86-only code) would also be fine. As long as both dom0-iommu=map-inclusive and iommu_inclusive_mapping are handled equally and the documentation is updated to reflect that. Thanks, Roger.
--- a/xen/drivers/passthrough/vtd/Makefile +++ b/xen/drivers/passthrough/vtd/Makefile @@ -1,5 +1,3 @@ -obj-$(CONFIG_X86) += x86/ - obj-y += iommu.o obj-y += dmar.o obj-y += utils.o --- a/xen/drivers/passthrough/vtd/extern.h +++ b/xen/drivers/passthrough/vtd/extern.h @@ -21,6 +21,7 @@ #define _VTD_EXTERN_H_ #include "dmar.h" +#include <xen/domain_page.h> #include <xen/keyhandler.h> #define VTDPREFIX "[VT-D]" @@ -68,8 +69,6 @@ struct acpi_rhsa_unit *drhd_to_rhsa(cons uint64_t alloc_pgtable_maddr(unsigned long npages, nodeid_t node); void free_pgtable_maddr(u64 maddr); -void *map_vtd_domain_page(u64 maddr); -void unmap_vtd_domain_page(const void *va); int domain_context_mapping_one(struct domain *domain, struct vtd_iommu *iommu, uint8_t bus, uint8_t devfn, const struct pci_dev *pdev, domid_t domid, @@ -79,6 +78,16 @@ int domain_context_unmap_one(struct doma int cf_check intel_iommu_get_reserved_device_memory( iommu_grdm_t *func, void *ctxt); +static inline void *map_vtd_domain_page(paddr_t maddr) +{ + return map_domain_page(_mfn(paddr_to_pfn(maddr))); +} + +static inline void unmap_vtd_domain_page(const void *va) +{ + unmap_domain_page(va); +} + unsigned int cf_check io_apic_read_remap_rte( unsigned int apic, unsigned int reg); void cf_check io_apic_write_remap_rte( --- a/xen/drivers/passthrough/vtd/x86/Makefile +++ /dev/null @@ -1 +0,0 @@ -obj-y += vtd.o --- a/xen/drivers/passthrough/vtd/x86/vtd.c +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright (c) 2008, Intel Corporation. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms and conditions of the GNU General Public License, - * version 2, as published by the Free Software Foundation. - * - * This program is distributed in the hope it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along with - * this program; If not, see <http://www.gnu.org/licenses/>. - * - * Copyright (C) Allen Kay <allen.m.kay@intel.com> - * Copyright (C) Weidong Han <weidong.han@intel.com> - */ - -#include <xen/param.h> -#include <xen/sched.h> -#include <xen/softirq.h> -#include <xen/domain_page.h> -#include <asm/paging.h> -#include <xen/iommu.h> -#include <xen/irq.h> -#include <xen/numa.h> -#include <asm/fixmap.h> -#include "../iommu.h" -#include "../dmar.h" -#include "../vtd.h" -#include "../extern.h" - -/* - * iommu_inclusive_mapping: when set, all memory below 4GB is included in dom0 - * 1:1 iommu mappings except xen and unusable regions. - */ -boolean_param("iommu_inclusive_mapping", iommu_hwdom_inclusive); - -void *map_vtd_domain_page(u64 maddr) -{ - return map_domain_page(_mfn(paddr_to_pfn(maddr))); -} - -void unmap_vtd_domain_page(const void *va) -{ - unmap_domain_page(va); -} --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -19,6 +19,7 @@ #include <xen/paging.h> #include <xen/guest_access.h> #include <xen/event.h> +#include <xen/param.h> #include <xen/softirq.h> #include <xen/vm_event.h> #include <xsm/xsm.h> @@ -36,6 +37,12 @@ bool __initdata iommu_superpages = true; enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full; +/* + * iommu_inclusive_mapping: When set, all memory below 4GB is included in dom0 + * 1:1 iommu mappings except xen and unusable regions. + */ +boolean_param("iommu_inclusive_mapping", iommu_hwdom_inclusive); + #ifdef CONFIG_PV /* Possible unfiltered LAPIC/MSI messages from untrusted sources? */ bool __read_mostly untrusted_msi;
..., thus allowing them to become static. There's nothing x86-specific about these functions anyway. Since only the "iommu_inclusive_mapping" parameter declaration would be left in the file, move that as well. There's nothing VT-d specific about it (anymore?): "dom0-iommu=map-inclusive" is similarly generic, and documentation also doesn't say anything. Signed-off-by: Jan Beulich <jbeulich@suse.com>