[PATCH] vfio/iommu_type1: Mantainance a counter for non_pinned_groups
With this counter, we never need to traverse all groups to update
pinned_scope of vfio_iommu.
Suggested-by: Alex Williamson
Signed-off-by: Keqian Zhu
---
drivers/vfio/vfio_iommu_type1.c | 40 +
1 file changed, 5 insertions(+), 35 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 0b4dedaa9128..bb4bbcc79101 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -73,7 +73,7 @@ struct vfio_iommu {
boolv2;
boolnesting;
booldirty_page_tracking;
- boolpinned_page_dirty_scope;
+ uint64_tnum_non_pinned_groups;
};
struct vfio_domain {
@@ -148,7 +148,6 @@ static int put_pfn(unsigned long pfn, int prot);
static struct vfio_group *vfio_iommu_find_iommu_group(struct vfio_iommu *iommu,
struct iommu_group *iommu_group);
-static void update_pinned_page_dirty_scope(struct vfio_iommu *iommu);
/*
* This code handles mapping and unmapping of user data buffers
* into DMA'ble space using the IOMMU
@@ -714,7 +713,7 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
group = vfio_iommu_find_iommu_group(iommu, iommu_group);
if (!group->pinned_page_dirty_scope) {
group->pinned_page_dirty_scope = true;
- update_pinned_page_dirty_scope(iommu);
+ iommu->num_non_pinned_groups--;
}
goto pin_done;
@@ -991,7 +990,7 @@ static int update_user_bitmap(u64 __user *bitmap, struct
vfio_iommu *iommu,
* mark all pages dirty if any IOMMU capable device is not able
* to report dirty pages and all pages are pinned and mapped.
*/
- if (!iommu->pinned_page_dirty_scope && dma->iommu_mapped)
+ if (iommu->num_non_pinned_groups && dma->iommu_mapped)
bitmap_set(dma->bitmap, 0, nbits);
if (shift) {
@@ -1622,33 +1621,6 @@ static struct vfio_group
*vfio_iommu_find_iommu_group(struct vfio_iommu *iommu,
return group;
}
-static void update_pinned_page_dirty_scope(struct vfio_iommu *iommu)
-{
- struct vfio_domain *domain;
- struct vfio_group *group;
-
- list_for_each_entry(domain, &iommu->domain_list, next) {
- list_for_each_entry(group, &domain->group_list, next) {
- if (!group->pinned_page_dirty_scope) {
- iommu->pinned_page_dirty_scope = false;
- return;
- }
- }
- }
-
- if (iommu->external_domain) {
- domain = iommu->external_domain;
- list_for_each_entry(group, &domain->group_list, next) {
- if (!group->pinned_page_dirty_scope) {
- iommu->pinned_page_dirty_scope = false;
- return;
- }
- }
- }
-
- iommu->pinned_page_dirty_scope = true;
-}
-
static bool vfio_iommu_has_sw_msi(struct list_head *group_resv_regions,
phys_addr_t *base)
{
@@ -2057,8 +2029,6 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
* addition of a dirty tracking group.
*/
group->pinned_page_dirty_scope = true;
- if (!iommu->pinned_page_dirty_scope)
- update_pinned_page_dirty_scope(iommu);
mutex_unlock(&iommu->lock);
return 0;
@@ -2188,7 +2158,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
* demotes the iommu scope until it declares itself dirty tracking
* capable via the page pinning interface.
*/
- iommu->pinned_page_dirty_scope = false;
+ iommu->num_non_pinned_groups++;
mutex_unlock(&iommu->lock);
vfio_iommu_resv_free(&group_resv_regions);
@@ -2416,7 +2386,7 @@ static void vfio_iommu_type1_detach_group(void
*iommu_data,
* to be promoted.
*/
if (update_dirty_scope)
- update_pinned_page_dirty_scope(iommu);
+ iommu->num_non_pinned_groups--;
mutex_unlock(&iommu->lock);
}
--
2.19.1
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 15/21] arm64: Add an aliasing facility for the idreg override
On Mon, 18 Jan 2021 13:18:39 +,
David Brazdil wrote:
>
> On Mon, Jan 18, 2021 at 09:45:27AM +, Marc Zyngier wrote:
> > In order to map the override of idregs to options that a user
> > can easily understand, let's introduce yet another option
> > array, which maps an option to the corresponding idreg options.
> >
> > Signed-off-by: Marc Zyngier
> Acked-by: David Brazdil
>
> > ---
> > arch/arm64/kernel/idreg-override.c | 20
> > 1 file changed, 20 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/idreg-override.c
> > b/arch/arm64/kernel/idreg-override.c
> > index 75d9845f489b..16bc8b3b93ae 100644
> > --- a/arch/arm64/kernel/idreg-override.c
> > +++ b/arch/arm64/kernel/idreg-override.c
> > @@ -37,6 +37,12 @@ static const struct reg_desc * const regs[] __initdata =
> > {
> > &mmfr1,
> > };
> >
> > +static const struct {
> > + const char * const alias;
> > + const char * const feature;
> > +} aliases[] __initdata = {
> > +};
> > +
> > static int __init find_field(const char *cmdline, const struct reg_desc
> > *reg,
> > int f, u64 *v)
> > {
> > @@ -80,6 +86,18 @@ static void __init match_options(const char *cmdline)
> > }
> > }
> >
> > +static __init void match_aliases(const char *cmdline)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(aliases); i++) {
> > + char *str = strstr(cmdline, aliases[i].alias);
> > +
> > + if ((str == cmdline || (str > cmdline && *(str - 1) == ' ')))
>
> nit: Extract to a 'cmdline_contains' helper? Took me a good few seconds to
> parse this in the previous patch. Giving it a name would help, and now it's
> also shared.
Good point. Adopted!
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 04/21] arm64: Provide an 'upgrade to VHE' stub hypercall
On Mon, 18 Jan 2021 11:25:16 +, David Brazdil wrote: > > On Mon, Jan 18, 2021 at 09:45:16AM +, Marc Zyngier wrote: > > As we are about to change the way a VHE system boots, let's > > provide the core helper, in the form of a stub hypercall that > > enables VHE and replicates the full EL1 context at EL2, thanks > > to EL1 and VHE-EL2 being extremely similar. > > > > On exception return, the kernel carries on at EL2. Fancy! > > > > Nothing calls this new hypercall yet, so no functional change. > > > > Signed-off-by: Marc Zyngier > > --- > > arch/arm64/include/asm/virt.h | 7 +++- > > arch/arm64/kernel/hyp-stub.S | 67 +-- > > 2 files changed, 71 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h > > index ee6a48df89d9..7379f35ae2c6 100644 > > --- a/arch/arm64/include/asm/virt.h > > +++ b/arch/arm64/include/asm/virt.h > > @@ -35,8 +35,13 @@ > > */ > > #define HVC_RESET_VECTORS 2 > > > > +/* > > + * HVC_VHE_RESTART - Upgrade the CPU from EL1 to EL2, if possible > > + */ > > +#define HVC_VHE_RESTART3 > > + > > /* Max number of HYP stub hypercalls */ > > -#define HVC_STUB_HCALL_NR 3 > > +#define HVC_STUB_HCALL_NR 4 > > > > /* Error returned when an invalid stub number is passed into x0 */ > > #define HVC_STUB_ERR 0xbadca11 > > diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S > > index 160f5881a0b7..fb12398b5c28 100644 > > --- a/arch/arm64/kernel/hyp-stub.S > > +++ b/arch/arm64/kernel/hyp-stub.S > > @@ -8,9 +8,9 @@ > > > > #include > > #include > > -#include > > > > #include > > +#include > > #include > > #include > > #include > > @@ -47,10 +47,13 @@ SYM_CODE_END(__hyp_stub_vectors) > > > > SYM_CODE_START_LOCAL(el1_sync) > > cmp x0, #HVC_SET_VECTORS > > - b.ne2f > > + b.ne1f > > msr vbar_el2, x1 > > b 9f > > > > +1: cmp x0, #HVC_VHE_RESTART > > + b.eqmutate_to_vhe > > + > > 2: cmp x0, #HVC_SOFT_RESTART > > b.ne3f > > mov x0, x2 > > @@ -70,6 +73,66 @@ SYM_CODE_START_LOCAL(el1_sync) > > eret > > SYM_CODE_END(el1_sync) > > > > +// nVHE? No way! Give me the real thing! > > +SYM_CODE_START_LOCAL(mutate_to_vhe) > > + // Sanity check: MMU *must* be off > > + mrs x0, sctlr_el2 > > + tbnzx0, #0, 1f > > + > > + // Needs to be VHE capable, obviously > > + mrs x0, id_aa64mmfr1_el1 > > + ubfxx0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4 > > + cbz x0, 1f > > nit: There is a HVC_STUB_ERR that you could return if these sanity > checks fail. The documentation also states that it should be > returned on error. Good point. I've now added it, but how the error can be handled is still up in the air. For now, I've decided to let the kernel continue its (probably doomed) course. Thanks, M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 18/21] arm64: Move "nokaslr" over to the early cpufeature infrastructure
On Mon, 18 Jan 2021 14:46:36 +,
David Brazdil wrote:
>
> On Mon, Jan 18, 2021 at 09:45:30AM +, Marc Zyngier wrote:
> > Given that the early cpufeature infrastructure has borrowed quite
> > a lot of code from the kaslr implementation, let's reimplement
> > the matching of the "nokaslr" option with it.
> >
> > Signed-off-by: Marc Zyngier
> Acked-by: David Brazdil
[...]
> > @@ -126,7 +95,7 @@ u64 __init kaslr_early_init(void)
> > * Check if 'nokaslr' appears on the command line, and
> > * return 0 if that is the case.
> > */
> > - if (is_kaslr_disabled_cmdline(fdt)) {
> > + if (kaslr_feature_val & kaslr_feature_mask & 0xf) {
>
> nit: Isn't the 0xf redundant here? You don't re-mask for VH either.
Actually, I do. See the two back to back ubfx that extract both the
mask and the feature. The "& 0xf" here serves the same purpose.
Is it redundant? At the moment, quite possibly. But since we have
space for 16 "features", this is an indication that we are only using
the first one. I expect that eventually, we'll use it for other
things.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 12/21] arm64: cpufeature: Add an early command-line cpufeature override facility
On Sat, 23 Jan 2021 13:43:52 +,
Catalin Marinas wrote:
>
> On Mon, Jan 18, 2021 at 09:45:24AM +, Marc Zyngier wrote:
> > +struct reg_desc {
> > + const char * const name;
> > + u64 * const val;
> > + u64 * const mask;
> > + struct {
> > + const char * const name;
> > + u8 shift;
> > + } fields[];
> > +};
>
> Sorry, I didn't see this earlier. Do we need to add all these consts
> here? So you want the pointers to be const but why is 'shift' special
> and not a const then? Is it modified later?
>
> Would this not work:
>
> struct reg_desc {
> const char *name;
> u64 *val;
> u64 *mask;
> struct {
> const char *name;
> u8 shift;
> } fields[];
> };
>
> > +static const struct reg_desc * const regs[] __initdata = {
>
> as we already declare the whole struct reg_desc pointers here as const.
> I may have confused myself...
It definitely is better. Specially given that we throw all of this
away right after boot, there is no harm in keeping it simple.
I've also renamed "reg_desc" to "ftr_set_desc", because we don't
always describe a register (like for kaslr).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 14/21] arm64: Honor VHE being disabled from the command-line
On Sat, 23 Jan 2021 14:07:53 +, Catalin Marinas wrote: > > On Mon, Jan 18, 2021 at 09:45:26AM +, Marc Zyngier wrote: > > diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S > > index 59820f9b8522..bbab2148a2a2 100644 > > --- a/arch/arm64/kernel/hyp-stub.S > > +++ b/arch/arm64/kernel/hyp-stub.S > > @@ -77,13 +77,24 @@ SYM_CODE_END(el1_sync) > > SYM_CODE_START_LOCAL(mutate_to_vhe) > > // Sanity check: MMU *must* be off > > mrs x0, sctlr_el2 > > - tbnzx0, #0, 1f > > + tbnzx0, #0, 2f > > > > // Needs to be VHE capable, obviously > > mrs x0, id_aa64mmfr1_el1 > > ubfxx0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4 > > - cbz x0, 1f > > + cbz x0, 2f > > > > + // Check whether VHE is disabled from the command line > > + adr_l x1, id_aa64mmfr1_val > > + ldr x0, [x1] > > + adr_l x1, id_aa64mmfr1_mask > > + ldr x1, [x1] > > + ubfxx0, x0, #ID_AA64MMFR1_VHE_SHIFT, #4 > > + ubfxx1, x1, #ID_AA64MMFR1_VHE_SHIFT, #4 > > + cbz x1, 1f > > + and x0, x0, x1 > > + cbz x0, 2f > > +1: > > I can see the advantage here in separate id_aa64mmfr1_val/mask but we > could use some asm offsets here and keep the pointer indirection simpler > in C code. You'd just need something like 'adr_l mmfr1_ovrd + VAL_OFFSET'. > > Anyway, if you have a strong preference for the current approach, leave > it as is. I've now moved over to a structure containing both val/mask, meaning that we only need to keep a single pointer around in the various feature descriptors. It certainly looks better. Thanks, M. -- Without deviation from the norm, progress is not possible. ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
