On 4/16/25 8:14 AM, Sairaj Kodilkar wrote:
+
+ /* DMA address translation support */
+ IOMMUNotifierFlag notifier_flags;
+ /* entry in list of Address spaces with registered notifiers */
+ QLIST_ENTRY(AMDVIAddressSpace) next;
+ /* DMA address translation active */
+ bool addr_translation;
I dont see any use of addr_translation in current patch.
maybe define it when you are really need this flag ?
ACK. I can move it to a later patch. My rationale was that this patch is
adding all the new structure members that will be needed, but it makes
sense to split it.
return 0;
}
@@ -1700,6 +1721,7 @@ static void amdvi_sysbus_realize(DeviceState
*dev, Error **errp)
static const Property amdvi_properties[] = {
DEFINE_PROP_BOOL("xtsup", AMDVIState, xtsup, false),
+ DEFINE_PROP_BOOL("dma-remap", AMDVIState, dma_remap, false),
};
Please add a description in commit message for this flag
Will change in next revision.
static const VMStateDescription vmstate_amdvi_sysbus = {
diff --git a/hw/i386/amd_iommu.h b/hw/i386/amd_iommu.h
index 28125130c6fc..e12ecade4baa 100644
--- a/hw/i386/amd_iommu.h
+++ b/hw/i386/amd_iommu.h
@@ -365,12 +365,18 @@ struct AMDVIState {
/* for each served device */
AMDVIAddressSpace **address_spaces[PCI_BUS_MAX];
+ /* list of address spaces with registered notifiers */
+ QLIST_HEAD(, AMDVIAddressSpace) amdvi_as_with_notifiers;
+
/* IOTLB */
GHashTable *iotlb;
/* Interrupt remapping */
bool ga_enabled;
bool xtsup;
+
+ /* DMA address translation */
+ bool dma_remap;
I think you should use this flag in the remapping path as well.
I am aware that you are using it later in this series to switch the
address space, but current patch will make things inconsistent for
emulated and vfio devices (possibly breaking the bisect).
The change in behavior happens only if the user explicitly sets
dma-remap=on property in the command line, which is why I made it off by
default.
To eliminate the possibility of using dma-remap=on before all the
infrastructure to support it is in place, I will move this patch and
[5/18] to the end of the series. Does that address your concern?
Also newer AMD IVRS format provides a HATDis bit (Table 96 in IOMMU
Specs [1]), informing the guest kernel that V1 page table is disabled in
the IOMMU.
Sounds like this mechanism could have been used until now to prevent the
scenario where we can have the guest request DMA remapping and the
vIOMMU doesn't have the capability. Seems moot now that we are actually
adding the capability.
Its good idea to set this bit in IVRS when dma_remap=false.
This way a "HATDis bit aware" guest can enable iommu.passthrough.
I'd need to research how to implement this, but isn't this enhancement
better added in separate series, since it also requires a companion
Linux change? I don't recall the Linux driver being "HATDis aware" (or
even HATS aware for that matter), but perhaps I am mistaken...
Also, is it a good idea to have default value for dma_remap=false ?
Consider the guest which is not aware of HATDis bit. Things will break
if such guest boots with iommu.passthrough=0 (recreating the pt=on
scenario).
That is not new, that is the current behavior that this series is trying
to fix by adding the missing functionality.
As far as the default value for dma-remap property, I think it must be
set to 0/false (i.e. current behavior unchanged) until we deem the DMA
remapping feature stable enough to be made available for guests.
On that topic, maybe it should be an experimental feature for now i.e.
"x-dma-remap".
[1] https://www.amd.com/content/dam/amd/en/documents/processor-tech-
docs/specifications/48882_IOMMU.pdf
Regards
Sairaj Kodilkar
PS: Sorry If I missed something here, I haven't progressed much in the
series.
No problem at all, the feedback is appreciated.
Thank you,
Alejandro
};
uint64_t amdvi_extended_feature_register(AMDVIState *s);
