Hi,
On 5/2/25 4:16 AM, Alejandro Jimenez wrote:
> Caution: External email. Do not open attachments or click links, unless this
> email comes from a known sender and you know the content is safe.
>
>
> A guest must issue an INVALIDATE_DEVTAB_ENTRY command after changing a
> Device Table entry (DTE) e.g. after attaching a device and setting up its
> DTE. When intercepting this event, determine if the DTE has been configured
> for paging or not, and toggle the appropriate memory regions to allow DMA
> address translation for the address space if needed. Requires dma-remap=on.
>
> Signed-off-by: Alejandro Jimenez <alejandro.j.jime...@oracle.com>
> ---
> hw/i386/amd_iommu.c | 78 +++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index a2df73062bf7..75a92067f35f 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -991,18 +991,92 @@ static void amdvi_switch_address_space_all(AMDVIState
> *s)
> }
> }
>
> +/*
> + * A guest driver must issue the INVALIDATE_DEVTAB_ENTRY command to the IOMMU
> + * after changing a Device Table entry. We can use this fact to detect when a
> + * Device Table entry is created for a device attached to a paging domain and
> + * enable the corresponding IOMMU memory region to allow for DMA translation
> if
> + * appropriate.
> + */
> +static void amdvi_update_addr_translation_mode(AMDVIState *s, uint16_t devid)
> +{
> + uint8_t bus_num, devfn, dte_mode;
> + AMDVIAddressSpace *as;
> + uint64_t dte[4] = { 0 };
> + IOMMUNotifier *n;
> + int ret;
> +
> + /*
> + * Convert the devid encoded in the command to a bus and devfn in
> + * order to retrieve the corresponding address space.
> + */
> + bus_num = PCI_BUS_NUM(devid);
> + devfn = devid & 0xff;
> +
> + /*
> + * The main buffer of size (AMDVIAddressSpace *) * (PCI_BUS_MAX) has
> already
> + * been allocated within AMDVIState, but must be careful to not access
> + * unallocated devfn.
> + */
> + if (!s->address_spaces[bus_num] || !s->address_spaces[bus_num][devfn]) {
> + return;
> + }
> + as = s->address_spaces[bus_num][devfn];
> +
> + ret = amdvi_as_to_dte(as, dte);
> +
> + if (!ret) {
> + dte_mode = (dte[0] >> AMDVI_DEV_MODE_RSHIFT) & AMDVI_DEV_MODE_MASK;
> + }
> +
> + if ((ret < 0) || (!ret && !dte_mode)) {
> + /*
> + * The DTE could not be retrieved, it is not valid, or it is not
> setup
> + * for paging. In either case, ensure that if paging was previously
> in
> + * use then invalidate all existing mappings and then switch to use
> the
> + * no_dma memory region.
> + */
If the DTE is malformed or could not be retrieved, wouldn't it be safer
to default to the DMA region rather than falling back to direct access?
Or am I missing something?
Thanks,
Ethan
> + if (as->addr_translation) {
> + as->addr_translation = false;
> +
> + IOMMU_NOTIFIER_FOREACH(n, &as->iommu) {
> + amdvi_address_space_unmap(as, n);
> + }
> + amdvi_switch_address_space(as);
> + }
> + } else if (!as->addr_translation) {
> + /*
> + * Installing a DTE that enables translation where it wasn't
> previously
> + * active. Activate the DMA memory region.
> + */
> + as->addr_translation = true;
> + amdvi_switch_address_space(as);
> + amdvi_address_space_sync(as);
> + }
> +}
> +
> /* log error without aborting since linux seems to be using reserved bits */
> static void amdvi_inval_devtab_entry(AMDVIState *s, uint64_t *cmd)
> {
> uint16_t devid = cpu_to_le16((uint16_t)extract64(cmd[0], 0, 16));
>
> + trace_amdvi_devtab_inval(PCI_BUS_NUM(devid), PCI_SLOT(devid),
> + PCI_FUNC(devid));
> +
> /* This command should invalidate internal caches of which there isn't */
> if (extract64(cmd[0], 16, 44) || cmd[1]) {
> amdvi_log_illegalcom_error(s, extract64(cmd[0], 60, 4),
> s->cmdbuf + s->cmdbuf_head);
> + return;
> + }
> +
> + /*
> + * When DMA remapping capability is enabled, check if updated DTE is
> setup
> + * for paging or not, and configure the corresponding memory regions.
> + */
> + if (s->dma_remap) {
> + amdvi_update_addr_translation_mode(s, devid);
> }
> - trace_amdvi_devtab_inval(PCI_BUS_NUM(devid), PCI_SLOT(devid),
> - PCI_FUNC(devid));
> }
>
> static void amdvi_complete_ppr(AMDVIState *s, uint64_t *cmd)
> --
> 2.43.5
>
