| Message ID | e0422c0127ebb402bb4f593d41571caf36b0864b.1701164432.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN] x86/guest_walk: address violations of MISRA C:2012 Rule 8.3 | expand |
On 28.11.2023 10:46, Federico Serafini wrote: > Uniform declaration and definition of guest_walk_tables() using > parameter name "pfec_walk": > this name highlights the connection with PFEC_* constants and it is > consistent with the use of the parameter within function body. > No functional change. > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> I'm curious what other x86 maintainers think. I for one don't like this, but not enough to object if others are happy. That said, there was earlier discussion (and perhaps even a patch), yet without a reference I don't think I can locate this among all the Misra bits and pieces. Jan > --- a/xen/arch/x86/include/asm/guest_pt.h > +++ b/xen/arch/x86/include/asm/guest_pt.h > @@ -422,7 +422,7 @@ static inline unsigned int guest_walk_to_page_order(const walk_t *gw) > > bool > guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > - unsigned long va, walk_t *gw, uint32_t pfec, > + unsigned long va, walk_t *gw, uint32_t pfec_walk, > gfn_t top_gfn, mfn_t top_mfn, void *top_map); > > /* Pretty-print the contents of a guest-walk */ > --- a/xen/arch/x86/mm/guest_walk.c > +++ b/xen/arch/x86/mm/guest_walk.c > @@ -69,7 +69,7 @@ static bool set_ad_bits(guest_intpte_t *guest_p, guest_intpte_t *walk_p, > */ > bool > guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > - unsigned long va, walk_t *gw, uint32_t walk, > + unsigned long va, walk_t *gw, uint32_t pfec_walk, > gfn_t top_gfn, mfn_t top_mfn, void *top_map) > { > struct domain *d = v->domain; > @@ -100,16 +100,17 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > * inputs to a guest walk, but a whole load of code currently passes in > * other PFEC_ constants. > */ > - walk &= (PFEC_implicit | PFEC_insn_fetch | PFEC_user_mode | PFEC_write_access); > + pfec_walk &= (PFEC_implicit | PFEC_insn_fetch | PFEC_user_mode | > + PFEC_write_access); > > /* Only implicit supervisor data accesses exist. */ > - ASSERT(!(walk & PFEC_implicit) || > - !(walk & (PFEC_insn_fetch | PFEC_user_mode))); > + ASSERT(!(pfec_walk & PFEC_implicit) || > + !(pfec_walk & (PFEC_insn_fetch | PFEC_user_mode))); > > perfc_incr(guest_walk); > memset(gw, 0, sizeof(*gw)); > gw->va = va; > - gw->pfec = walk & (PFEC_user_mode | PFEC_write_access); > + gw->pfec = pfec_walk & (PFEC_user_mode | PFEC_write_access); > > /* > * PFEC_insn_fetch is only reported if NX or SMEP are enabled. Hardware > @@ -117,7 +118,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > * rights. > */ > if ( guest_nx_enabled(v) || guest_smep_enabled(v) ) > - gw->pfec |= (walk & PFEC_insn_fetch); > + gw->pfec |= (pfec_walk & PFEC_insn_fetch); > > #if GUEST_PAGING_LEVELS >= 3 /* PAE or 64... */ > #if GUEST_PAGING_LEVELS >= 4 /* 64-bit only... */ > @@ -399,7 +400,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > * N.B. In the case that the walk ended with a superpage, the fabricated > * gw->l1e contains the appropriate leaf pkey. > */ > - if ( !(walk & PFEC_insn_fetch) && > + if ( !(pfec_walk & PFEC_insn_fetch) && > ((ar & _PAGE_USER) ? guest_pku_enabled(v) > : guest_pks_enabled(v)) ) > { > @@ -408,8 +409,8 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > unsigned int pk_ar = (pkr >> (pkey * PKEY_WIDTH)) & (PKEY_AD | PKEY_WD); > > if ( (pk_ar & PKEY_AD) || > - ((walk & PFEC_write_access) && (pk_ar & PKEY_WD) && > - ((walk & PFEC_user_mode) || guest_wp_enabled(v))) ) > + ((pfec_walk & PFEC_write_access) && (pk_ar & PKEY_WD) && > + ((pfec_walk & PFEC_user_mode) || guest_wp_enabled(v))) ) > { > gw->pfec |= PFEC_prot_key; > goto out; > @@ -417,17 +418,17 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > } > #endif > > - if ( (walk & PFEC_insn_fetch) && (ar & _PAGE_NX_BIT) ) > + if ( (pfec_walk & PFEC_insn_fetch) && (ar & _PAGE_NX_BIT) ) > /* Requested an instruction fetch and found NX? Fail. */ > goto out; > > - if ( walk & PFEC_user_mode ) /* Requested a user acess. */ > + if ( pfec_walk & PFEC_user_mode ) /* Requested a user acess. */ > { > if ( !(ar & _PAGE_USER) ) > /* Got a supervisor walk? Unconditional fail. */ > goto out; > > - if ( (walk & PFEC_write_access) && !(ar & _PAGE_RW) ) > + if ( (pfec_walk & PFEC_write_access) && !(ar & _PAGE_RW) ) > /* Requested a write and only got a read? Fail. */ > goto out; > } > @@ -435,18 +436,18 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > { > if ( ar & _PAGE_USER ) /* Got a user walk. */ > { > - if ( (walk & PFEC_insn_fetch) && guest_smep_enabled(v) ) > + if ( (pfec_walk & PFEC_insn_fetch) && guest_smep_enabled(v) ) > /* User insn fetch and smep? Fail. */ > goto out; > > - if ( !(walk & PFEC_insn_fetch) && guest_smap_enabled(v) && > - ((walk & PFEC_implicit) || > + if ( !(pfec_walk & PFEC_insn_fetch) && guest_smap_enabled(v) && > + ((pfec_walk & PFEC_implicit) || > !(guest_cpu_user_regs()->eflags & X86_EFLAGS_AC)) ) > /* User data access and smap? Fail. */ > goto out; > } > > - if ( (walk & PFEC_write_access) && !(ar & _PAGE_RW) && > + if ( (pfec_walk & PFEC_write_access) && !(ar & _PAGE_RW) && > guest_wp_enabled(v) ) > /* Requested a write, got a read, and CR0.WP is set? Fail. */ > goto out; > @@ -468,7 +469,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > > case 1: > if ( set_ad_bits(&l1p[guest_l1_table_offset(va)].l1, &gw->l1e.l1, > - (walk & PFEC_write_access)) ) > + (pfec_walk & PFEC_write_access)) ) > { > paging_mark_dirty(d, gw->l1mfn); > hvmemul_write_cache(v, l1gpa, &gw->l1e, sizeof(gw->l1e)); > @@ -476,7 +477,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > /* Fallthrough */ > case 2: > if ( set_ad_bits(&l2p[guest_l2_table_offset(va)].l2, &gw->l2e.l2, > - (walk & PFEC_write_access) && leaf_level == 2) ) > + (pfec_walk & PFEC_write_access) && leaf_level == 2) ) > { > paging_mark_dirty(d, gw->l2mfn); > hvmemul_write_cache(v, l2gpa, &gw->l2e, sizeof(gw->l2e)); > @@ -485,7 +486,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, > #if GUEST_PAGING_LEVELS == 4 /* 64-bit only... */ > case 3: > if ( set_ad_bits(&l3p[guest_l3_table_offset(va)].l3, &gw->l3e.l3, > - (walk & PFEC_write_access) && leaf_level == 3) ) > + (pfec_walk & PFEC_write_access) && leaf_level == 3) ) > { > paging_mark_dirty(d, gw->l3mfn); > hvmemul_write_cache(v, l3gpa, &gw->l3e, sizeof(gw->l3e));
On 28/11/23 14:00, Jan Beulich wrote: > On 28.11.2023 10:46, Federico Serafini wrote: >> Uniform declaration and definition of guest_walk_tables() using >> parameter name "pfec_walk": >> this name highlights the connection with PFEC_* constants and it is >> consistent with the use of the parameter within function body. >> No functional change. >> >> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> > > I'm curious what other x86 maintainers think. I for one don't like this, > but not enough to object if others are happy. That said, there was earlier > discussion (and perhaps even a patch), yet without a reference I don't > think I can locate this among all the Misra bits and pieces. > The last thread about guest_walk_tables(): https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg02055.html Do you have any suggestions on how to handle this?
On 28/11/2023 1:00 pm, Jan Beulich wrote: > On 28.11.2023 10:46, Federico Serafini wrote: >> Uniform declaration and definition of guest_walk_tables() using >> parameter name "pfec_walk": >> this name highlights the connection with PFEC_* constants and it is >> consistent with the use of the parameter within function body. >> No functional change. >> >> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> > I'm curious what other x86 maintainers think. I for one don't like this, > but not enough to object if others are happy. That said, there was earlier > discussion (and perhaps even a patch), yet without a reference I don't > think I can locate this among all the Misra bits and pieces. I looked at this and wanted a bit of time to think. Sadly, this code is half way through some cleanup, which started before speculation and will continue in my copious free time. It's wrong to be passing PFEC_* constants, and that's why I renamed pfec -> walk the last time I was fixing security bugs here (indeed, passing the wrong constant here *was* the security issue). I missed the prototype while fixing the implementation. At some point, PFEC_* will no longer be passed in. Therefore I'd far rather this was a one-line change for the declaration changing pfec -> walk. As it stands, you're effectively reverting a correction I made. Thanks, ~Andrew
On 28.11.2023 14:17, Andrew Cooper wrote: > On 28/11/2023 1:00 pm, Jan Beulich wrote: >> On 28.11.2023 10:46, Federico Serafini wrote: >>> Uniform declaration and definition of guest_walk_tables() using >>> parameter name "pfec_walk": >>> this name highlights the connection with PFEC_* constants and it is >>> consistent with the use of the parameter within function body. >>> No functional change. >>> >>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> >> I'm curious what other x86 maintainers think. I for one don't like this, >> but not enough to object if others are happy. That said, there was earlier >> discussion (and perhaps even a patch), yet without a reference I don't >> think I can locate this among all the Misra bits and pieces. > > I looked at this and wanted a bit of time to think. > > Sadly, this code is half way through some cleanup, which started before > speculation and will continue in my copious free time. > > It's wrong to be passing PFEC_* constants, and that's why I renamed pfec > -> walk the last time I was fixing security bugs here (indeed, passing > the wrong constant here *was* the security issue). I missed the > prototype while fixing the implementation. > > At some point, PFEC_* will no longer be passed in. > > Therefore I'd far rather this was a one-line change for the declaration > changing pfec -> walk. So that was what Federico originally had. Did you see my response at https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00122.html ? While I certainly agree with your plans (as far as I understand them), doing as you suggest would make it harder to spot what values are correct to pass to the function today. With a suitable new set of constants and perhaps even a proper typedef, such confusion would likely not arise. Jan
On 28.11.2023 14:11, Federico Serafini wrote: > On 28/11/23 14:00, Jan Beulich wrote: >> On 28.11.2023 10:46, Federico Serafini wrote: >>> Uniform declaration and definition of guest_walk_tables() using >>> parameter name "pfec_walk": >>> this name highlights the connection with PFEC_* constants and it is >>> consistent with the use of the parameter within function body. >>> No functional change. >>> >>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> >> >> I'm curious what other x86 maintainers think. I for one don't like this, >> but not enough to object if others are happy. That said, there was earlier >> discussion (and perhaps even a patch), yet without a reference I don't >> think I can locate this among all the Misra bits and pieces. >> > > The last thread about guest_walk_tables(): > > https://lists.xenproject.org/archives/html/xen-devel/2023-10/msg02055.html > > Do you have any suggestions on how to handle this? This simply will need solving - see my reply to Andrew sent a minute ago. Personally the best route I would see is to leave things as they are for now, hoping that Andrew's indicated code change will arrive in not too distant a future. Jan
On 28/11/23 14:43, Jan Beulich wrote: > On 28.11.2023 14:17, Andrew Cooper wrote: >> On 28/11/2023 1:00 pm, Jan Beulich wrote: >>> On 28.11.2023 10:46, Federico Serafini wrote: >>>> Uniform declaration and definition of guest_walk_tables() using >>>> parameter name "pfec_walk": >>>> this name highlights the connection with PFEC_* constants and it is >>>> consistent with the use of the parameter within function body. >>>> No functional change. >>>> >>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> >>> I'm curious what other x86 maintainers think. I for one don't like this, >>> but not enough to object if others are happy. That said, there was earlier >>> discussion (and perhaps even a patch), yet without a reference I don't >>> think I can locate this among all the Misra bits and pieces. >> >> I looked at this and wanted a bit of time to think. >> >> Sadly, this code is half way through some cleanup, which started before >> speculation and will continue in my copious free time. >> >> It's wrong to be passing PFEC_* constants, and that's why I renamed pfec >> -> walk the last time I was fixing security bugs here (indeed, passing >> the wrong constant here *was* the security issue). I missed the >> prototype while fixing the implementation. >> >> At some point, PFEC_* will no longer be passed in. >> >> Therefore I'd far rather this was a one-line change for the declaration >> changing pfec -> walk. > > So that was what Federico originally had. Did you see my response at > https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00122.html ? > While I certainly agree with your plans (as far as I understand them), > doing as you suggest would make it harder to spot what values are correct > to pass to the function today. With a suitable new set of constants and > perhaps even a proper typedef, such confusion would likely not arise. Thanks to both for the information. I take this opportunity to inform that we are really close to the end with Rule 8.3 for x86, this is the situation: - do_multicall(), Stefano sent a patch; - guest_walk_tables(), Andrew will take care of it; - xenmem_add_to_physmap_one(), this is the last one. For the latter, I see you (x86) share the declaration with ARM, where "gfn" is used for the last parameter instead of "gpfn". Do you agree in changing the name in the definition from "gpfn" to "gfn"? If you agree, do you have any suggestions on how to rename the local variable "gfn"? Following your suggestions, I can do the renaming in two steps to prevent bad things to happening.
On 29.11.2023 10:35, Federico Serafini wrote: > I take this opportunity to inform that we are really close to the end > with Rule 8.3 for x86, this is the situation: > - do_multicall(), Stefano sent a patch; > - guest_walk_tables(), Andrew will take care of it; > - xenmem_add_to_physmap_one(), this is the last one. > > For the latter, I see you (x86) share the declaration with ARM, > where "gfn" is used for the last parameter instead of "gpfn". > Do you agree in changing the name in the definition from "gpfn" > to "gfn"? Yes. > If you agree, do you have any suggestions on how to rename > the local variable "gfn"? Considering its exclusive use for the XENMAPSPACE_gmfn case, I'm inclined to suggest "gmfn", despite this being a term we generally try to get rid of. Maybe Andrew or Roger have a better suggestion. Along with renaming "gpfn" it would be nice (for consistency) to also rename "old_gpfn" at the same time. Jan
On 29/11/23 12:09, Jan Beulich wrote: > On 29.11.2023 10:35, Federico Serafini wrote: >> I take this opportunity to inform that we are really close to the end >> with Rule 8.3 for x86, this is the situation: >> - do_multicall(), Stefano sent a patch; >> - guest_walk_tables(), Andrew will take care of it; >> - xenmem_add_to_physmap_one(), this is the last one. >> >> For the latter, I see you (x86) share the declaration with ARM, >> where "gfn" is used for the last parameter instead of "gpfn". >> Do you agree in changing the name in the definition from "gpfn" >> to "gfn"? > > Yes. > >> If you agree, do you have any suggestions on how to rename >> the local variable "gfn"? > > Considering its exclusive use for the XENMAPSPACE_gmfn case, I'm inclined > to suggest "gmfn", despite this being a term we generally try to get rid > of. Maybe Andrew or Roger have a better suggestion. > > Along with renaming "gpfn" it would be nice (for consistency) to also > rename "old_gpfn" at the same time. Thank you.
diff --git a/xen/arch/x86/include/asm/guest_pt.h b/xen/arch/x86/include/asm/guest_pt.h index bc312343cd..5edf687dce 100644 --- a/xen/arch/x86/include/asm/guest_pt.h +++ b/xen/arch/x86/include/asm/guest_pt.h @@ -422,7 +422,7 @@ static inline unsigned int guest_walk_to_page_order(const walk_t *gw) bool guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, - unsigned long va, walk_t *gw, uint32_t pfec, + unsigned long va, walk_t *gw, uint32_t pfec_walk, gfn_t top_gfn, mfn_t top_mfn, void *top_map); /* Pretty-print the contents of a guest-walk */ diff --git a/xen/arch/x86/mm/guest_walk.c b/xen/arch/x86/mm/guest_walk.c index fe7393334f..20a19bd7e2 100644 --- a/xen/arch/x86/mm/guest_walk.c +++ b/xen/arch/x86/mm/guest_walk.c @@ -69,7 +69,7 @@ static bool set_ad_bits(guest_intpte_t *guest_p, guest_intpte_t *walk_p, */ bool guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, - unsigned long va, walk_t *gw, uint32_t walk, + unsigned long va, walk_t *gw, uint32_t pfec_walk, gfn_t top_gfn, mfn_t top_mfn, void *top_map) { struct domain *d = v->domain; @@ -100,16 +100,17 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, * inputs to a guest walk, but a whole load of code currently passes in * other PFEC_ constants. */ - walk &= (PFEC_implicit | PFEC_insn_fetch | PFEC_user_mode | PFEC_write_access); + pfec_walk &= (PFEC_implicit | PFEC_insn_fetch | PFEC_user_mode | + PFEC_write_access); /* Only implicit supervisor data accesses exist. */ - ASSERT(!(walk & PFEC_implicit) || - !(walk & (PFEC_insn_fetch | PFEC_user_mode))); + ASSERT(!(pfec_walk & PFEC_implicit) || + !(pfec_walk & (PFEC_insn_fetch | PFEC_user_mode))); perfc_incr(guest_walk); memset(gw, 0, sizeof(*gw)); gw->va = va; - gw->pfec = walk & (PFEC_user_mode | PFEC_write_access); + gw->pfec = pfec_walk & (PFEC_user_mode | PFEC_write_access); /* * PFEC_insn_fetch is only reported if NX or SMEP are enabled. Hardware @@ -117,7 +118,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, * rights. */ if ( guest_nx_enabled(v) || guest_smep_enabled(v) ) - gw->pfec |= (walk & PFEC_insn_fetch); + gw->pfec |= (pfec_walk & PFEC_insn_fetch); #if GUEST_PAGING_LEVELS >= 3 /* PAE or 64... */ #if GUEST_PAGING_LEVELS >= 4 /* 64-bit only... */ @@ -399,7 +400,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, * N.B. In the case that the walk ended with a superpage, the fabricated * gw->l1e contains the appropriate leaf pkey. */ - if ( !(walk & PFEC_insn_fetch) && + if ( !(pfec_walk & PFEC_insn_fetch) && ((ar & _PAGE_USER) ? guest_pku_enabled(v) : guest_pks_enabled(v)) ) { @@ -408,8 +409,8 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, unsigned int pk_ar = (pkr >> (pkey * PKEY_WIDTH)) & (PKEY_AD | PKEY_WD); if ( (pk_ar & PKEY_AD) || - ((walk & PFEC_write_access) && (pk_ar & PKEY_WD) && - ((walk & PFEC_user_mode) || guest_wp_enabled(v))) ) + ((pfec_walk & PFEC_write_access) && (pk_ar & PKEY_WD) && + ((pfec_walk & PFEC_user_mode) || guest_wp_enabled(v))) ) { gw->pfec |= PFEC_prot_key; goto out; @@ -417,17 +418,17 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, } #endif - if ( (walk & PFEC_insn_fetch) && (ar & _PAGE_NX_BIT) ) + if ( (pfec_walk & PFEC_insn_fetch) && (ar & _PAGE_NX_BIT) ) /* Requested an instruction fetch and found NX? Fail. */ goto out; - if ( walk & PFEC_user_mode ) /* Requested a user acess. */ + if ( pfec_walk & PFEC_user_mode ) /* Requested a user acess. */ { if ( !(ar & _PAGE_USER) ) /* Got a supervisor walk? Unconditional fail. */ goto out; - if ( (walk & PFEC_write_access) && !(ar & _PAGE_RW) ) + if ( (pfec_walk & PFEC_write_access) && !(ar & _PAGE_RW) ) /* Requested a write and only got a read? Fail. */ goto out; } @@ -435,18 +436,18 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, { if ( ar & _PAGE_USER ) /* Got a user walk. */ { - if ( (walk & PFEC_insn_fetch) && guest_smep_enabled(v) ) + if ( (pfec_walk & PFEC_insn_fetch) && guest_smep_enabled(v) ) /* User insn fetch and smep? Fail. */ goto out; - if ( !(walk & PFEC_insn_fetch) && guest_smap_enabled(v) && - ((walk & PFEC_implicit) || + if ( !(pfec_walk & PFEC_insn_fetch) && guest_smap_enabled(v) && + ((pfec_walk & PFEC_implicit) || !(guest_cpu_user_regs()->eflags & X86_EFLAGS_AC)) ) /* User data access and smap? Fail. */ goto out; } - if ( (walk & PFEC_write_access) && !(ar & _PAGE_RW) && + if ( (pfec_walk & PFEC_write_access) && !(ar & _PAGE_RW) && guest_wp_enabled(v) ) /* Requested a write, got a read, and CR0.WP is set? Fail. */ goto out; @@ -468,7 +469,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, case 1: if ( set_ad_bits(&l1p[guest_l1_table_offset(va)].l1, &gw->l1e.l1, - (walk & PFEC_write_access)) ) + (pfec_walk & PFEC_write_access)) ) { paging_mark_dirty(d, gw->l1mfn); hvmemul_write_cache(v, l1gpa, &gw->l1e, sizeof(gw->l1e)); @@ -476,7 +477,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, /* Fallthrough */ case 2: if ( set_ad_bits(&l2p[guest_l2_table_offset(va)].l2, &gw->l2e.l2, - (walk & PFEC_write_access) && leaf_level == 2) ) + (pfec_walk & PFEC_write_access) && leaf_level == 2) ) { paging_mark_dirty(d, gw->l2mfn); hvmemul_write_cache(v, l2gpa, &gw->l2e, sizeof(gw->l2e)); @@ -485,7 +486,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m, #if GUEST_PAGING_LEVELS == 4 /* 64-bit only... */ case 3: if ( set_ad_bits(&l3p[guest_l3_table_offset(va)].l3, &gw->l3e.l3, - (walk & PFEC_write_access) && leaf_level == 3) ) + (pfec_walk & PFEC_write_access) && leaf_level == 3) ) { paging_mark_dirty(d, gw->l3mfn); hvmemul_write_cache(v, l3gpa, &gw->l3e, sizeof(gw->l3e));
Uniform declaration and definition of guest_walk_tables() using parameter name "pfec_walk": this name highlights the connection with PFEC_* constants and it is consistent with the use of the parameter within function body. No functional change. Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> --- xen/arch/x86/include/asm/guest_pt.h | 2 +- xen/arch/x86/mm/guest_walk.c | 39 +++++++++++++++-------------- 2 files changed, 21 insertions(+), 20 deletions(-)