| Message ID | 20190603160350.29806-12-julien.grall@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | xen/arm: Properly disable M2P on Arm | expand |
>>> On 03.06.19 at 18:03, <julien.grall@arm.com> wrote: > p2m_pt_audit_p2m() has one place where the same message may be printed > twice via printk and P2M_PRINTK. > > Remove the one printed using printk to stay consistent with the rest of > the code. > > Take the opportunity to reflow the format of P2M_PRINTK. Hmm, yes, but ... > --- a/xen/arch/x86/mm/p2m-pt.c > +++ b/xen/arch/x86/mm/p2m-pt.c > @@ -1041,9 +1041,8 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m) > if ( m2pfn != (gfn + i2) ) > { > pmbad++; > - P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx" > - " -> gfn %#lx\n", gfn+i2, mfn+i2, > - m2pfn); > + P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx -> gfn %#lx\n", > + gfn + i2, mfn + i2, m2pfn); ... you re-flow an unrelated (but similar) one while ... > @@ -1108,8 +1107,6 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m) > !p2m_is_shared(type) ) > { > pmbad++; > - printk("mismatch: gfn %#lx -> mfn %#lx" > - " -> gfn %#lx\n", gfn, mfn, m2pfn); > P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx" > " -> gfn %#lx\n", gfn, mfn, m2pfn); ... you leave alone this one. I don't mind touching the other one, but this one surely wants touching then as well. And if you touch that other one, then I think for consistency you should also touch the 3rd one (between the two). Jan
Hi Jan, On 05/06/2019 11:43, Jan Beulich wrote: >>>> On 03.06.19 at 18:03, <julien.grall@arm.com> wrote: >> p2m_pt_audit_p2m() has one place where the same message may be printed >> twice via printk and P2M_PRINTK. >> >> Remove the one printed using printk to stay consistent with the rest of >> the code. >> >> Take the opportunity to reflow the format of P2M_PRINTK. > > Hmm, yes, but ... This is a mistake when I wrote the patch/rebase. > >> --- a/xen/arch/x86/mm/p2m-pt.c >> +++ b/xen/arch/x86/mm/p2m-pt.c >> @@ -1041,9 +1041,8 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m) >> if ( m2pfn != (gfn + i2) ) >> { >> pmbad++; >> - P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx" >> - " -> gfn %#lx\n", gfn+i2, mfn+i2, >> - m2pfn); >> + P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx -> gfn %#lx\n", >> + gfn + i2, mfn + i2, m2pfn); > > ... you re-flow an unrelated (but similar) one while ... > >> @@ -1108,8 +1107,6 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m) >> !p2m_is_shared(type) ) >> { >> pmbad++; >> - printk("mismatch: gfn %#lx -> mfn %#lx" >> - " -> gfn %#lx\n", gfn, mfn, m2pfn); >> P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx" >> " -> gfn %#lx\n", gfn, mfn, m2pfn); > > ... you leave alone this one. I don't mind touching the other > one, but this one surely wants touching then as well. And if > you touch that other one, then I think for consistency you > should also touch the 3rd one (between the two). I will only re-flow this message. Cheers,
diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c index cafc9f299b..84ddc1834b 100644 --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -1041,9 +1041,8 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m) if ( m2pfn != (gfn + i2) ) { pmbad++; - P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx" - " -> gfn %#lx\n", gfn+i2, mfn+i2, - m2pfn); + P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx -> gfn %#lx\n", + gfn + i2, mfn + i2, m2pfn); BUG(); } gfn += 1 << (L3_PAGETABLE_SHIFT - PAGE_SHIFT); @@ -1108,8 +1107,6 @@ long p2m_pt_audit_p2m(struct p2m_domain *p2m) !p2m_is_shared(type) ) { pmbad++; - printk("mismatch: gfn %#lx -> mfn %#lx" - " -> gfn %#lx\n", gfn, mfn, m2pfn); P2M_PRINTK("mismatch: gfn %#lx -> mfn %#lx" " -> gfn %#lx\n", gfn, mfn, m2pfn); BUG();
p2m_pt_audit_p2m() has one place where the same message may be printed twice via printk and P2M_PRINTK. Remove the one printed using printk to stay consistent with the rest of the code. Take the opportunity to reflow the format of P2M_PRINTK. Signed-off-by: Julien Grall <julien.grall@arm.com> --- Changes in v3: - Patch added --- xen/arch/x86/mm/p2m-pt.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)