[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 2/6] amd-vi: set IOMMU page table levels based on guest reported paddr width
On 05.12.2023 16:11, Roger Pau Monné wrote: > On Tue, Dec 05, 2023 at 03:32:20PM +0100, Jan Beulich wrote: >> On 04.12.2023 10:43, Roger Pau Monne wrote: >>> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c >>> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c >>> @@ -359,21 +359,17 @@ int __read_mostly amd_iommu_min_paging_mode = 1; >>> static int cf_check amd_iommu_domain_init(struct domain *d) >>> { >>> struct domain_iommu *hd = dom_iommu(d); >>> + int pglvl = amd_iommu_get_paging_mode( >>> + PFN_DOWN(1UL << paging_max_paddr_bits(d))); >> >> This is a function in the paging subsystem, i.e. generally inapplicable >> to system domains (specifically DomIO). If this is to remain this way, >> the function would imo need to gain a warning. Yet better would imo be >> if the function was avoided for system domains. > > I have to admit I'm confused, won't systems domains return > paging_mode_hap(d) == false, and thus fallback to using paddr_bits > (host paddr width?). True, but that check lives inside the function. > I can avoid such domains calling into paging_max_paddr_bits() but it > seems redundant, and would just be duplicated logic for a case that > paging_max_paddr_bits() already handles correctly AFAICT. Hence why I suggested a comment (warning) as alternative. > Would it be better for me to rename paging_max_paddr_bits() to > domain_max_paddr_bits() and move it to asm/domain.h? Maybe. I'm not sure exactly why the function was introduced where it is and under the name it has. It sole present caller is in cpu-policy.c, so either name/placement would look good to me. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |