Re: [PATCH] intel-iommu: Set status bit after operation completed

[Michael S. Tsirkin]

On Thu, Mar 09, 2023 at 05:23:19PM +0800, Zhenzhong Duan wrote:
> According to SDM 11.4.4.2 Global Status Register:
> "This field is cleared by hardware when software sets the SRTP field in the
> Global Command register. This field is set by hardware when hardware
> completes the ‘Set Root Table Pointer’ operation using the value provided
> in the Root Table Address register"
> 
> Follow above spec to clear then set RTPS after finish all works, this way
> helps avoiding potential race with guest kernel. Though linux kernel is
> single threaded in writing GCMD_REG and checking GSTS_REG.
> 
> Same reasion for GSTS_REG.TES
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


So I am dropping this?

> ---
>  hw/i386/intel_iommu.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index faade7def8..7cba1945a3 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2312,11 +2312,12 @@ static void vtd_handle_gcmd_qie(IntelIOMMUState *s, 
> bool en)
>  /* Set Root Table Pointer */
>  static void vtd_handle_gcmd_srtp(IntelIOMMUState *s)
>  {
> +    vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_RTPS, 0);
>      vtd_root_table_setup(s);
> -    /* Ok - report back to driver */
> -    vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_RTPS);
>      vtd_reset_caches(s);
>      vtd_address_space_refresh_all(s);
> +    /* Ok - report back to driver */
> +    vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_RTPS);
>  }
>  
>  /* Set Interrupt Remap Table Pointer */
> @@ -2338,19 +2339,22 @@ static void vtd_handle_gcmd_te(IntelIOMMUState *s, 
> bool en)
>  
>      if (en) {
>          s->dmar_enabled = true;
> -        /* Ok - report back to driver */
> -        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_TES);
>      } else {
>          s->dmar_enabled = false;
>  
>          /* Clear the index of Fault Recording Register */
>          s->next_frcd_reg = 0;
> -        /* Ok - report back to driver */
> -        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_TES, 0);
>      }
>  
>      vtd_reset_caches(s);
>      vtd_address_space_refresh_all(s);
> +
> +    /* Ok - report back to driver */
> +    if (en) {
> +        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, 0, VTD_GSTS_TES);
> +    } else {
> +        vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_TES, 0);
> +    }
>  }
>  
>  /* Handle Interrupt Remap Enable/Disable */
> -- 
> 2.25.1