|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [PATCH v7 08/11] IOMMU/VT-d: wire common device reserved memory API
> From: Marek Marczykowski-Górecki
> Sent: Saturday, September 17, 2022 10:51 AM
>
> Re-use rmrr= parameter handling code to handle common device reserved
> memory.
>
> Signed-off-by: Marek Marczykowski-Górecki
> <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> Changes in v3:
> - make MAX_USER_RMRR_PAGES applicable only to user-configured RMRR
> ---
> xen/drivers/passthrough/vtd/dmar.c | 201 +++++++++++++++++-------------
> 1 file changed, 119 insertions(+), 82 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/dmar.c
> b/xen/drivers/passthrough/vtd/dmar.c
> index 367304c8739c..3df5f6b69719 100644
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -861,111 +861,139 @@ static struct user_rmrr __initdata
> user_rmrrs[MAX_USER_RMRR];
>
> /* Macro for RMRR inclusive range formatting. */
> #define ERMRRU_FMT "[%lx-%lx]"
> -#define ERMRRU_ARG(eru) eru.base_pfn, eru.end_pfn
> +#define ERMRRU_ARG base_pfn, end_pfn
> +
> +static int __init add_one_user_rmrr(unsigned long base_pfn,
> + unsigned long end_pfn,
> + unsigned int dev_count,
> + uint32_t *sbdf);
Just move the function here then no need of a declaration.
>
> static int __init add_user_rmrr(void)
> {
> + unsigned int i;
> + int ret;
> +
> + for ( i = 0; i < nr_rmrr; i++ )
> + {
> + ret = add_one_user_rmrr(user_rmrrs[i].base_pfn,
> + user_rmrrs[i].end_pfn,
> + user_rmrrs[i].dev_count,
> + user_rmrrs[i].sbdf);
> + if ( ret < 0 )
> + return ret;
> + }
> + return 0;
> +}
> +
> +/* Returns 1 on success, 0 when ignoring and < 0 on error. */
I'm not sure the point of 'ignoring'. If user specifies a wrong RMRR
range (overlap, mfn invalid, etc.) just return an error similar to
-ENOMEM. Ignoring a user-specified range implies that something
would potentially get broken hence better fail it early.
> +static int __init add_one_user_rmrr(unsigned long base_pfn,
> + unsigned long end_pfn,
> + unsigned int dev_count,
> + uint32_t *sbdf)
> +{
> struct acpi_rmrr_unit *rmrr, *rmrru;
> - unsigned int idx, seg, i;
> - unsigned long base, end;
> + unsigned int idx, seg;
> + unsigned long base_iter;
> bool overlap;
>
> - for ( i = 0; i < nr_rmrr; i++ )
> + if ( iommu_verbose )
> + printk(XENLOG_DEBUG VTDPREFIX
> + "Adding RMRR for %d device ([0]: %#x) range "ERMRRU_FMT"\n",
> + dev_count, sbdf[0], ERMRRU_ARG);
> +
> + if ( base_pfn > end_pfn )
> {
> - base = user_rmrrs[i].base_pfn;
> - end = user_rmrrs[i].end_pfn;
> + printk(XENLOG_ERR VTDPREFIX
> + "Invalid RMRR Range "ERMRRU_FMT"\n",
> + ERMRRU_ARG);
> + return 0;
> + }
>
> - if ( base > end )
> + overlap = false;
> + list_for_each_entry(rmrru, &acpi_rmrr_units, list)
> + {
> + if ( pfn_to_paddr(base_pfn) <= rmrru->end_address &&
> + rmrru->base_address <= pfn_to_paddr(end_pfn) )
> {
> printk(XENLOG_ERR VTDPREFIX
> - "Invalid RMRR Range "ERMRRU_FMT"\n",
> - ERMRRU_ARG(user_rmrrs[i]));
> - continue;
> + "Overlapping RMRRs: "ERMRRU_FMT" and [%lx-%lx]\n",
> + ERMRRU_ARG,
> + paddr_to_pfn(rmrru->base_address),
> + paddr_to_pfn(rmrru->end_address));
> + overlap = true;
> + break;
> }
> + }
> + /* Don't add overlapping RMRR. */
> + if ( overlap )
> + return 0;
>
> - if ( (end - base) >= MAX_USER_RMRR_PAGES )
> + base_iter = base_pfn;
> + do
> + {
> + if ( !mfn_valid(_mfn(base_iter)) )
> {
> printk(XENLOG_ERR VTDPREFIX
> - "RMRR range "ERMRRU_FMT" exceeds "\
> - __stringify(MAX_USER_RMRR_PAGES)" pages\n",
> - ERMRRU_ARG(user_rmrrs[i]));
> - continue;
> + "Invalid pfn in RMRR range "ERMRRU_FMT"\n",
> + ERMRRU_ARG);
> + break;
> }
> + } while ( base_iter++ < end_pfn );
>
> - overlap = false;
> - list_for_each_entry(rmrru, &acpi_rmrr_units, list)
> - {
> - if ( pfn_to_paddr(base) <= rmrru->end_address &&
> - rmrru->base_address <= pfn_to_paddr(end) )
> - {
> - printk(XENLOG_ERR VTDPREFIX
> - "Overlapping RMRRs: "ERMRRU_FMT" and [%lx-%lx]\n",
> - ERMRRU_ARG(user_rmrrs[i]),
> - paddr_to_pfn(rmrru->base_address),
> - paddr_to_pfn(rmrru->end_address));
> - overlap = true;
> - break;
> - }
> - }
> - /* Don't add overlapping RMRR. */
> - if ( overlap )
> - continue;
> + /* Invalid pfn in range as the loop ended before end_pfn was reached. */
> + if ( base_iter <= end_pfn )
> + return 0;
>
> - do
> - {
> - if ( !mfn_valid(_mfn(base)) )
> - {
> - printk(XENLOG_ERR VTDPREFIX
> - "Invalid pfn in RMRR range "ERMRRU_FMT"\n",
> - ERMRRU_ARG(user_rmrrs[i]));
> - break;
> - }
> - } while ( base++ < end );
> + rmrr = xzalloc(struct acpi_rmrr_unit);
> + if ( !rmrr )
> + return -ENOMEM;
>
> - /* Invalid pfn in range as the loop ended before end_pfn was reached.
> */
> - if ( base <= end )
> - continue;
> + rmrr->scope.devices = xmalloc_array(u16, dev_count);
> + if ( !rmrr->scope.devices )
> + {
> + xfree(rmrr);
> + return -ENOMEM;
> + }
>
> - rmrr = xzalloc(struct acpi_rmrr_unit);
> - if ( !rmrr )
> - return -ENOMEM;
> + seg = 0;
> + for ( idx = 0; idx < dev_count; idx++ )
> + {
> + rmrr->scope.devices[idx] = sbdf[idx];
> + seg |= PCI_SEG(sbdf[idx]);
> + }
> + if ( seg != PCI_SEG(sbdf[0]) )
> + {
> + printk(XENLOG_ERR VTDPREFIX
> + "Segments are not equal for RMRR range "ERMRRU_FMT"\n",
> + ERMRRU_ARG);
> + scope_devices_free(&rmrr->scope);
> + xfree(rmrr);
> + return 0;
> + }
>
> - rmrr->scope.devices = xmalloc_array(u16, user_rmrrs[i].dev_count);
> - if ( !rmrr->scope.devices )
> - {
> - xfree(rmrr);
> - return -ENOMEM;
> - }
> + rmrr->segment = seg;
> + rmrr->base_address = pfn_to_paddr(base_pfn);
> + /* Align the end_address to the end of the page */
> + rmrr->end_address = pfn_to_paddr(end_pfn) | ~PAGE_MASK;
> + rmrr->scope.devices_cnt = dev_count;
>
> - seg = 0;
> - for ( idx = 0; idx < user_rmrrs[i].dev_count; idx++ )
> - {
> - rmrr->scope.devices[idx] = user_rmrrs[i].sbdf[idx];
> - seg |= PCI_SEG(user_rmrrs[i].sbdf[idx]);
> - }
> - if ( seg != PCI_SEG(user_rmrrs[i].sbdf[0]) )
> - {
> - printk(XENLOG_ERR VTDPREFIX
> - "Segments are not equal for RMRR range "ERMRRU_FMT"\n",
> - ERMRRU_ARG(user_rmrrs[i]));
> - scope_devices_free(&rmrr->scope);
> - xfree(rmrr);
> - continue;
> - }
> + if ( register_one_rmrr(rmrr) )
> + printk(XENLOG_ERR VTDPREFIX
> + "Could not register RMMR range "ERMRRU_FMT"\n",
> + ERMRRU_ARG);
>
> - rmrr->segment = seg;
> - rmrr->base_address = pfn_to_paddr(user_rmrrs[i].base_pfn);
> - /* Align the end_address to the end of the page */
> - rmrr->end_address = pfn_to_paddr(user_rmrrs[i].end_pfn) |
> ~PAGE_MASK;
> - rmrr->scope.devices_cnt = user_rmrrs[i].dev_count;
> + return 1;
> +}
>
> - if ( register_one_rmrr(rmrr) )
> - printk(XENLOG_ERR VTDPREFIX
> - "Could not register RMMR range "ERMRRU_FMT"\n",
> - ERMRRU_ARG(user_rmrrs[i]));
> - }
> +static int __init cf_check add_one_extra_rmrr(xen_pfn_t start, xen_ulong_t
> nr, u32 id, void *ctxt)
> +{
> + u32 sbdf_array[] = { id };
> + return add_one_user_rmrr(start, start+nr, 1, sbdf_array);
> +}
>
> - return 0;
> +static int __init add_extra_rmrr(void)
> +{
> + return iommu_get_extra_reserved_device_memory(add_one_extra_rmrr,
> NULL);
> }
>
> #include <asm/tboot.h>
> @@ -1010,7 +1038,7 @@ int __init acpi_dmar_init(void)
> {
> iommu_init_ops = &intel_iommu_init_ops;
>
> - return add_user_rmrr();
> + return add_user_rmrr() || add_extra_rmrr();
> }
>
> return ret;
> @@ -1108,6 +1136,15 @@ static int __init cf_check parse_rmrr_param(const
> char *str)
> else
> end = start;
>
> + if ( (end - start) >= MAX_USER_RMRR_PAGES )
> + {
> + printk(XENLOG_ERR VTDPREFIX
> + "RMRR range "ERMRRU_FMT" exceeds "\
> + __stringify(MAX_USER_RMRR_PAGES)" pages\n",
> + start, end);
> + return -E2BIG;
> + }
> +
why moving this error check out of add_one_user_rmrr()? I didn't
get why it's special from other checks there, e.g. having base>end...
> user_rmrrs[nr_rmrr].base_pfn = start;
> user_rmrrs[nr_rmrr].end_pfn = end;
>
> --
> git-series 0.9.1
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |