On Mon, Sep 16, 2024 at 05:18:01PM +0000, Suravee Suthikulpanit wrote:
....
+ if (!(ptr->data[0] & DTE_FLAG_V)) {
+ /* Existing DTE is not valid. */
+ write_upper(ptr, new);
+ write_lower(ptr, new);
+ iommu_flush_sync_dte(iommu, dev_data->devid);
+ } else if (!(new->data[0] & DTE_FLAG_V)) {
+ /* Existing DTE is valid. New DTE is not valid. */
+ write_lower(ptr, new);
+ write_upper(ptr, new);
+ iommu_flush_sync_dte(iommu, dev_data->devid);
+ } else {
+ /* Existing & new DTEs are valid. */
+ if (!FIELD_GET(DTE_FLAG_GV, ptr->data[0])) {
+ /* Existing DTE has no guest page table. */
+ write_upper(ptr, new);
+ write_lower(ptr, new);
+ iommu_flush_sync_dte(iommu, dev_data->devid);
+ } else if (!FIELD_GET(DTE_FLAG_GV, new->data[0])) {
+ /*
+ * Existing DTE has guest page table,
+ * new DTE has no guest page table,
+ */
+ write_lower(ptr, new);
+ write_upper(ptr, new);
+ iommu_flush_sync_dte(iommu, dev_data->devid);
+ } else {
+ /*
+ * Existing DTE has guest page table,
+ * new DTE has guest page table.
+ */
+ struct dev_table_entry clear = {};
+
+ /* First disable DTE */
+ write_lower(ptr, &clear);
+ iommu_flush_sync_dte(iommu, dev_data->devid);
+
+ /* Then update DTE */
+ write_upper(ptr, new);
+ write_lower(ptr, new);
+ iommu_flush_sync_dte(iommu, dev_data->devid);
+ }
There is one branch missing where GV is valid in both and the [1]
doesn't change. Ie atomic replace of a GCR3 table.
And maybe this will need more branches later for the viommu stuff?
But otherwise yes this captures what is needed just fine.
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
@@ -1256,6 +1342,16 @@ static int iommu_flush_dte(struct amd_iommu *iommu, u16 devid)
+int iommu_flush_sync_dte(struct amd_iommu *iommu, u16 devid)
+{
+ int ret;
+
+ ret = iommu_flush_dte(iommu, devid);
+ if (!ret)
+ iommu_completion_wait(iommu);
+ return ret;
+}
Maybe this doesn't need to return an error since we can't handle
failure to flush DTE tables..