Hi Peter,
On 10/8/18 8:47 AM, Peter Xu wrote:
> There are two callers for vtd_sync_shadow_page_table_range(): one
> provided a valid context entry and one not. Move that fetching
> operation into the caller vtd_sync_shadow_page_table() where we need to
> fetch the context entry.
>
> Meanwhile, remove the error_report_once() directly since we're already
> tracing all the error cases in the previous call. Instead, return error
> number back to caller. This will not change anything functional since
> callers are dropping it after all.
>
> We do this move majorly because we want to do something more later in
> vtd_sync_shadow_page_table().
>
> Signed-off-by: Peter Xu <pet...@redhat.com>
Reviewed-by: Eric Auger <eric.au...@redhat.com>
Thanks
Eric
> ---
> dtc | 2 +-
> hw/i386/intel_iommu.c | 41 +++++++++++++----------------------------
> 2 files changed, 14 insertions(+), 29 deletions(-)
>
> diff --git a/dtc b/dtc
> index 88f18909db..e54388015a 160000
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 88f18909db731a627456f26d779445f84e449536
> +Subproject commit e54388015af1fb4bf04d0bca99caba1074d9cc42
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 3dfada19a6..d6b4f8705d 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1035,7 +1035,6 @@ static int vtd_sync_shadow_page_hook(IOMMUTLBEntry
> *entry,
> return 0;
> }
>
> -/* If context entry is NULL, we'll try to fetch it on our own. */
> static int vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as,
> VTDContextEntry *ce,
> hwaddr addr, hwaddr size)
> @@ -1047,39 +1046,25 @@ static int
> vtd_sync_shadow_page_table_range(VTDAddressSpace *vtd_as,
> .notify_unmap = true,
> .aw = s->aw_bits,
> .as = vtd_as,
> + .domain_id = VTD_CONTEXT_ENTRY_DID(ce->hi),
> };
> - VTDContextEntry ce_cache;
> - int ret;
> -
> - if (ce) {
> - /* If the caller provided context entry, use it */
> - ce_cache = *ce;
> - } else {
> - /* If the caller didn't provide ce, try to fetch */
> - ret = vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
> - vtd_as->devfn, &ce_cache);
> - if (ret) {
> - /*
> - * This should not really happen, but in case it happens,
> - * we just skip the sync for this time. After all we even
> - * don't have the root table pointer!
> - */
> - error_report_once("%s: invalid context entry for bus 0x%x"
> - " devfn 0x%x",
> - __func__, pci_bus_num(vtd_as->bus),
> - vtd_as->devfn);
> - return 0;
> - }
> - }
>
> - info.domain_id = VTD_CONTEXT_ENTRY_DID(ce_cache.hi);
> -
> - return vtd_page_walk(&ce_cache, addr, addr + size, &info);
> + return vtd_page_walk(ce, addr, addr + size, &info);
> }
>
> static int vtd_sync_shadow_page_table(VTDAddressSpace *vtd_as)
> {
> - return vtd_sync_shadow_page_table_range(vtd_as, NULL, 0, UINT64_MAX);
> + int ret;
> + VTDContextEntry ce;
> +
> + ret = vtd_dev_to_context_entry(vtd_as->iommu_state,
> + pci_bus_num(vtd_as->bus),
> + vtd_as->devfn, &ce);
> + if (ret) {
> + return ret;
> + }
> +
> + return vtd_sync_shadow_page_table_range(vtd_as, &ce, 0, UINT64_MAX);
> }
>
> /*
>
