From: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
To: Dave Hansen <dave.hansen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
"H . Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>,
Toshi Kani <toshi.kani-VXdhtT5mjnY@public.gmane.org>,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Sai Praneeth Prakhya
<sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
Borislav Petkov <bp-l3A5Bk7waGM@public.gmane.org>
Subject: Re: [PATCH v2 1/5] x86/mm/pageattr: Ensure cpa->pfn only contains page frame numbers
Date: Tue, 17 Nov 2015 09:44:10 +0000 [thread overview]
Message-ID: <20151117094410.GA2727@codeblueprint.co.uk> (raw)
In-Reply-To: <5649FCA1.1000600-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Mon, 16 Nov, at 07:56:17AM, Dave Hansen wrote:
> I'm glad you're looking at this. It obviously needed some love. :)
>
> On 11/14/2015 02:00 PM, Matt Fleming wrote:
> > + npages = (_end - _text) >> PAGE_SHIFT;
> > + text = __pa(_text);
> > + pfn = text >> PAGE_SHIFT;
> > +
> > + if (kernel_map_pages_in_pgd(pgd, pfn, text, npages, 0)) {
> > + pr_err("Failed to map kernel text 1:1\n");
> > + return 1;
> > + }
>
> Are _end and _text guaranteed to be aligned? If not, I think the
> calculation might be wrong. Just for fun, imagine that _end=0xfff and
> _text=0x1001. npages would be 0.
Bugger. Good catch, thanks.
> Some other code like set_kernel_text_rw() does alignment on _text.
>
> One nit is that there's quite a bit going on here, like rearranging the
> phys_stack arithmetic ordering that is far beyond just simplifying the
> paddr vs. pfn issue, but that isn't called out in the changelog at all.
Yeah, the phys_stack hunk actually slipped into this patch by
accident. It ensures the stack is mapped into the EFI page tables.
I'll split this out.
> Your fixes all look correct to me, fwiw.
Thanks! If you could respond to the next version with an ACK or
Reviewed-by tag, that'd be great.
WARNING: multiple messages have this Message-ID (diff)
From: Matt Fleming <matt@codeblueprint.co.uk>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
"H . Peter Anvin" <hpa@zytor.com>, Toshi Kani <toshi.kani@hp.com>,
linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org,
Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
Borislav Petkov <bp@suse.de>
Subject: Re: [PATCH v2 1/5] x86/mm/pageattr: Ensure cpa->pfn only contains page frame numbers
Date: Tue, 17 Nov 2015 09:44:10 +0000 [thread overview]
Message-ID: <20151117094410.GA2727@codeblueprint.co.uk> (raw)
In-Reply-To: <5649FCA1.1000600@intel.com>
On Mon, 16 Nov, at 07:56:17AM, Dave Hansen wrote:
> I'm glad you're looking at this. It obviously needed some love. :)
>
> On 11/14/2015 02:00 PM, Matt Fleming wrote:
> > + npages = (_end - _text) >> PAGE_SHIFT;
> > + text = __pa(_text);
> > + pfn = text >> PAGE_SHIFT;
> > +
> > + if (kernel_map_pages_in_pgd(pgd, pfn, text, npages, 0)) {
> > + pr_err("Failed to map kernel text 1:1\n");
> > + return 1;
> > + }
>
> Are _end and _text guaranteed to be aligned? If not, I think the
> calculation might be wrong. Just for fun, imagine that _end=0xfff and
> _text=0x1001. npages would be 0.
Bugger. Good catch, thanks.
> Some other code like set_kernel_text_rw() does alignment on _text.
>
> One nit is that there's quite a bit going on here, like rearranging the
> phys_stack arithmetic ordering that is far beyond just simplifying the
> paddr vs. pfn issue, but that isn't called out in the changelog at all.
Yeah, the phys_stack hunk actually slipped into this patch by
accident. It ensures the stack is mapped into the EFI page tables.
I'll split this out.
> Your fixes all look correct to me, fwiw.
Thanks! If you could respond to the next version with an ACK or
Reviewed-by tag, that'd be great.
next prev parent reply other threads:[~2015-11-17 9:44 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-14 22:00 [GIT PULL v2 0/5] EFI page table isolation Matt Fleming
2015-11-14 22:00 ` Matt Fleming
[not found] ` <1447538451-5793-1-git-send-email-matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2015-11-14 22:00 ` [PATCH v2 1/5] x86/mm/pageattr: Ensure cpa->pfn only contains page frame numbers Matt Fleming
2015-11-14 22:00 ` Matt Fleming
[not found] ` <1447538451-5793-2-git-send-email-matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2015-11-16 15:56 ` Dave Hansen
2015-11-16 15:56 ` Dave Hansen
[not found] ` <5649FCA1.1000600-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-11-17 9:44 ` Matt Fleming [this message]
2015-11-17 9:44 ` Matt Fleming
2015-11-16 20:19 ` Thomas Gleixner
2015-11-16 20:19 ` Thomas Gleixner
2015-11-16 21:20 ` Borislav Petkov
[not found] ` <20151116212042.GE20435-fF5Pk5pvG8Y@public.gmane.org>
2015-11-16 21:48 ` Thomas Gleixner
2015-11-16 21:48 ` Thomas Gleixner
2015-11-17 8:50 ` Thomas Gleixner
2015-11-17 8:50 ` Thomas Gleixner
2015-11-17 9:45 ` Matt Fleming
2015-11-17 9:45 ` Matt Fleming
[not found] ` <20151117094509.GB2727-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2015-11-18 8:14 ` Ingo Molnar
2015-11-18 8:14 ` Ingo Molnar
[not found] ` <20151118081423.GA23844-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-11-20 12:01 ` Matt Fleming
2015-11-20 12:01 ` Matt Fleming
2015-11-14 22:00 ` [PATCH 2/5] x86/efi: Map RAM into the identity page table for mixed mode Matt Fleming
2015-11-14 22:00 ` Matt Fleming
2015-11-14 22:00 ` [PATCH v2 3/5] x86/efi: Hoist page table switching code into efi_call_virt() Matt Fleming
2015-11-14 22:00 ` Matt Fleming
2015-11-14 22:00 ` [PATCH v2 4/5] x86/efi: Build our own page table structures Matt Fleming
2015-11-14 22:00 ` Matt Fleming
2015-11-14 22:00 ` [PATCH 5/5] Documentation/x86: Update EFI memory region description Matt Fleming
2015-11-14 22:00 ` Matt Fleming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151117094410.GA2727@codeblueprint.co.uk \
--to=matt-mf/unelci9gs6ibeejttw/xrex20p6io@public.gmane.org \
--cc=bp-l3A5Bk7waGM@public.gmane.org \
--cc=dave.hansen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
--cc=hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org \
--cc=linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=sai.praneeth.prakhya-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
--cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
--cc=toshi.kani-VXdhtT5mjnY@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.