Re: [PATCH v2 08/17] intel_iommu: Set accessed and dirty bits during first stage translation

[Yi Liu <yi.l.liu@intel.com>]

On 2024/8/5 14:27, Zhenzhong Duan wrote:
> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> 
> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/i386/intel_iommu_internal.h |  3 +++
>   hw/i386/intel_iommu.c          | 24 ++++++++++++++++++++++++
>   2 files changed, 27 insertions(+)
> 
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 668583aeca..7786ef7624 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -324,6 +324,7 @@ typedef enum VTDFaultReason {
>   
>       /* Output address in the interrupt address range for scalable mode */
>       VTD_FR_SM_INTERRUPT_ADDR = 0x87,
> +    VTD_FR_FS_BIT_UPDATE_FAILED = 0x91, /* SFS.10 */
>       VTD_FR_MAX,                 /* Guard */
>   } VTDFaultReason;
>   
> @@ -549,6 +550,8 @@ typedef struct VTDRootEntry VTDRootEntry;
>   /* Masks for First Level Paging Entry */
>   #define VTD_FL_P                    1ULL
>   #define VTD_FL_RW_MASK              (1ULL << 1)
> +#define VTD_FL_A                    0x20
> +#define VTD_FL_D                    0x40
>   
>   /* Second Level Page Translation Pointer*/
>   #define VTD_SM_PASID_ENTRY_SLPTPTR     (~0xfffULL)
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 6121cca4cd..3c2ceed284 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1822,6 +1822,7 @@ static const bool vtd_qualified_faults[] = {
>       [VTD_FR_PASID_TABLE_ENTRY_INV] = true,
>       [VTD_FR_SM_INTERRUPT_ADDR] = true,
>       [VTD_FR_FS_NON_CANONICAL] = true,
> +    [VTD_FR_FS_BIT_UPDATE_FAILED] = true,
>       [VTD_FR_MAX] = false,
>   };
>   
> @@ -1939,6 +1940,20 @@ static bool vtd_iova_fl_check_canonical(IntelIOMMUState *s, uint64_t iova,
>               );
>   }
>   
> +static MemTxResult vtd_set_flag_in_pte(dma_addr_t base_addr, uint32_t index,
> +                                       uint64_t pte, uint64_t flag)
> +{
> +    if (pte & flag) {
> +        return MEMTX_OK;
> +    }
> +    pte |= flag;
> +    pte = cpu_to_le64(pte);
> +    return dma_memory_write(&address_space_memory,
> +                            base_addr + index * sizeof(pte),
> +                            &pte, sizeof(pte),
> +                            MEMTXATTRS_UNSPECIFIED);

Can we ensure this write is atomic? A/D bit setting should be atomic from
guest p.o.v.

> +}
> +
>   /*
>    * Given the @iova, get relevant @flptep. @flpte_level will be the last level
>    * of the translation, can be used for deciding the size of large page.
> @@ -1990,7 +2005,16 @@ static int vtd_iova_to_flpte(IntelIOMMUState *s, VTDContextEntry *ce,
>               return -VTD_FR_PAGING_ENTRY_RSVD;
>           }
>   
> +        if (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_A) != MEMTX_OK) {
> +            return -VTD_FR_FS_BIT_UPDATE_FAILED;
> +        }
> +
>           if (vtd_is_last_pte(flpte, level)) {
> +            if (is_write &&
> +                (vtd_set_flag_in_pte(addr, offset, flpte, VTD_FL_D) !=
> +                                                                    MEMTX_OK)) {
> +                    return -VTD_FR_FS_BIT_UPDATE_FAILED;
> +            }
>               *flptep = flpte;
>               *flpte_level = level;
>               return 0;

-- 
Regards,
Yi Liu