On Fri, Sep 24, 2021 at 11:41:14AM +0200, Jan Beulich wrote:
> In order to be able to insert/remove super-pages we need to allow
> callers of the walking function to specify at which point to stop the
> walk. (For now at least gcc will instantiate just a variant of the
> function with the parameter eliminated, so effectively no change to
> generated code as far as the parameter addition goes.)
>
> Instead of merely adjusting a BUG_ON() condition, convert it into an
> error return - there's no reason to crash the entire host in that case.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>
> --- a/xen/drivers/passthrough/amd/iommu_map.c
> +++ b/xen/drivers/passthrough/amd/iommu_map.c
> @@ -178,7 +178,8 @@ void __init iommu_dte_add_device_entry(s
> * page tables.
> */
> static int iommu_pde_from_dfn(struct domain *d, unsigned long dfn,
> - unsigned long *pt_mfn, bool map)
> + unsigned int target, unsigned long *pt_mfn,
> + bool map)
> {
> union amd_iommu_pte *pde, *next_table_vaddr;
> unsigned long next_table_mfn;
> @@ -189,7 +190,8 @@ static int iommu_pde_from_dfn(struct dom
> table = hd->arch.amd.root_table;
> level = hd->arch.amd.paging_mode;
>
> - BUG_ON( table == NULL || level < 1 || level > 6 );
> + if ( !table || target < 1 || level < target || level > 6 )
> + return 1;
I would consider adding an ASSERT_UNREACHABLE here, since there should
be no callers passing those parameters, and we shouldn't be
introducing new ones. Unless you believe there could be valid callers
passing level < target parameter.
>
> /*
> * A frame number past what the current page tables can represent can't
> @@ -200,7 +202,7 @@ static int iommu_pde_from_dfn(struct dom
>
> next_table_mfn = mfn_x(page_to_mfn(table));
>
> - while ( level > 1 )
> + while ( level > target )
> {
> unsigned int next_level = level - 1;
There's a comment at the bottom of iommu_pde_from_dfn that needs to be
adjusted to no longer explicitly mention level 1.
With that adjusted:
Reviewed-by: Roger Pau Monné <roger....@citrix.com>
FWIW, I always get confused with AMD and shadow code using level 1 to
denote the smaller page size level while Intel uses 0.
Thanks, Roger.
