Re: [PATCH v2 14/18] IOMMU: fold flush-all hook into "flush one"

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Sep 24, 2021 at 11:53:59AM +0200, Jan Beulich wrote:
> Having a separate flush-all hook has always been puzzling me some. We
> will want to be able to force a full flush via accumulated flush flags
> from the map/unmap functions. Introduce a respective new flag and fold
> all flush handling to use the single remaining hook.
> 
> Note that because of the respective comments in SMMU and IPMMU-VMSA
> code, I've folded the two prior hook functions into one. For SMMU-v3,
> which lacks a comment towards incapable hardware, I've left both
> functions in place on the assumption that selective and full flushes
> will eventually want separating.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Just one nit I think.

> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -731,18 +731,21 @@ static int __must_check iommu_flush_iotl
>                                                  unsigned long page_count,
>                                                  unsigned int flush_flags)
>  {
> -    ASSERT(page_count && !dfn_eq(dfn, INVALID_DFN));
> -    ASSERT(flush_flags);
> +    if ( flush_flags & IOMMU_FLUSHF_all )
> +    {
> +        dfn = INVALID_DFN;
> +        page_count = 0;

Don't we expect callers to already pass an invalid dfn and a 0 page
count when doing a full flush?

In the equivalent AMD code you didn't set those for the FLUSHF_all
case.

Thanks, Roger.