* Re: [PATCH] Documentation: DMA-API: fix a function name of max_mapping_size
From: Christoph Hellwig @ 2019-06-07 8:34 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Christoph Hellwig, corbet@lwn.net, jroedel@suse.de,
m.szyprowski@samsung.com, linux-doc@vger.kernel.org,
iommu@lists.linux-foundation.org
In-Reply-To: <TYAPR01MB3102C6CCC204DAAA6570FD25D8100@TYAPR01MB3102.jpnprd01.prod.outlook.com>
On Fri, Jun 07, 2019 at 08:19:08AM +0000, Yoshihiro Shimoda wrote:
> Hi Christoph,
>
> > From: Christoph Hellwig, Sent: Friday, June 7, 2019 5:08 PM
> >
> > Looks good. And it seems like you've also found the solution to
> > your usb storage problem, but I'm going to post the variant I just
> > hacked up nevertheless.
>
> Thank you for your reply! I think this API is related to my problem,
> but I don't have any actual solution (a patch) for now. So, I'll wait
> for your patch!
Turns out it isn't as simple as I thought, as there doesn't seem to
be an easy way to get to the struct device used for DMA mapping
from USB drivers. I'll need to think a bit more how to handle that
best.
^ permalink raw reply
* RE: [PATCH] Documentation: DMA-API: fix a function name of max_mapping_size
From: Yoshihiro Shimoda @ 2019-06-07 8:19 UTC (permalink / raw)
To: Christoph Hellwig
Cc: corbet@lwn.net, jroedel@suse.de, m.szyprowski@samsung.com,
linux-doc@vger.kernel.org, iommu@lists.linux-foundation.org
In-Reply-To: <20190607080814.GA10303@lst.de>
Hi Christoph,
> From: Christoph Hellwig, Sent: Friday, June 7, 2019 5:08 PM
>
> Looks good. And it seems like you've also found the solution to
> your usb storage problem, but I'm going to post the variant I just
> hacked up nevertheless.
Thank you for your reply! I think this API is related to my problem,
but I don't have any actual solution (a patch) for now. So, I'll wait
for your patch!
Best regards,
Yoshihiro Shimoda
^ permalink raw reply
* [PATCH] Documentation: fix typo CLOCK_MONONOTNIC_COARSE
From: Aurelien Thierry @ 2019-06-07 8:07 UTC (permalink / raw)
To: Jonathan Corbet, Arnd Bergmann, John Stultz, Thomas Gleixner,
Linus Walleij, Randy Dunlap
Cc: Aurelien Thierry, linux-doc, trivial
Fix typo in documentation file timekeeping.rst: CLOCK_MONONOTNIC_COARSE
should be CLOCK_MONOTONIC_COARSE.
Signed-off-by: Aurelien Thierry <aurelien.thierry@quoscient.io>
---
Documentation/core-api/timekeeping.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/core-api/timekeeping.rst
b/Documentation/core-api/timekeeping.rst
index 93cbeb9daec0..5f87d9c8b04d 100644
--- a/Documentation/core-api/timekeeping.rst
+++ b/Documentation/core-api/timekeeping.rst
@@ -111,7 +111,7 @@ Some additional variants exist for more specialized
cases:
void ktime_get_coarse_raw_ts64( struct timespec64 * )
These are quicker than the non-coarse versions, but less accurate,
- corresponding to CLOCK_MONONOTNIC_COARSE and CLOCK_REALTIME_COARSE
+ corresponding to CLOCK_MONOTONIC_COARSE and CLOCK_REALTIME_COARSE
in user space, along with the equivalent boottime/tai/raw
timebase not available in user space.
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v7 03/14] x86/cet/ibt: Add IBT legacy code bitmap setup function
From: Peter Zijlstra @ 2019-06-07 8:08 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200926.4029-4-yu-cheng.yu@intel.com>
On Thu, Jun 06, 2019 at 01:09:15PM -0700, Yu-cheng Yu wrote:
> Indirect Branch Tracking (IBT) provides an optional legacy code bitmap
> that allows execution of legacy, non-IBT compatible library by an
> IBT-enabled application. When set, each bit in the bitmap indicates
> one page of legacy code.
>
> The bitmap is allocated and setup from the application.
> +int cet_setup_ibt_bitmap(unsigned long bitmap, unsigned long size)
> +{
> + u64 r;
> +
> + if (!current->thread.cet.ibt_enabled)
> + return -EINVAL;
> +
> + if (!PAGE_ALIGNED(bitmap) || (size > TASK_SIZE_MAX))
> + return -EINVAL;
> +
> + current->thread.cet.ibt_bitmap_addr = bitmap;
> + current->thread.cet.ibt_bitmap_size = size;
> +
> + /*
> + * Turn on IBT legacy bitmap.
> + */
> + modify_fpu_regs_begin();
> + rdmsrl(MSR_IA32_U_CET, r);
> + r |= (MSR_IA32_CET_LEG_IW_EN | bitmap);
> + wrmsrl(MSR_IA32_U_CET, r);
> + modify_fpu_regs_end();
> +
> + return 0;
> +}
So you just program a random user supplied address into the hardware.
What happens if there's not actually anything at that address or the
user munmap()s the data after doing this?
^ permalink raw reply
* Re: [PATCH] Documentation: DMA-API: fix a function name of max_mapping_size
From: Christoph Hellwig @ 2019-06-07 8:08 UTC (permalink / raw)
To: Yoshihiro Shimoda; +Cc: corbet, jroedel, hch, m.szyprowski, linux-doc, iommu
In-Reply-To: <1559893633-6852-1-git-send-email-yoshihiro.shimoda.uh@renesas.com>
Looks good. And it seems like you've also found the solution to
your usb storage problem, but I'm going to post the variant I just
hacked up nevertheless.
^ permalink raw reply
* Re: [PATCH v7 07/14] x86/cet/ibt: Add arch_prctl functions for IBT
From: Peter Zijlstra @ 2019-06-07 8:07 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200926.4029-8-yu-cheng.yu@intel.com>
On Thu, Jun 06, 2019 at 01:09:19PM -0700, Yu-cheng Yu wrote:
> +static int handle_bitmap(unsigned long arg2)
> +{
> + unsigned long addr, size;
> +
> + if (get_user(addr, (unsigned long __user *)arg2) ||
> + get_user(size, (unsigned long __user *)arg2 + 1))
> + return -EFAULT;
> +
> + return cet_setup_ibt_bitmap(addr, size);
> +}
> + /*
> + * Allocate legacy bitmap and return address & size to user.
> + */
> + case ARCH_X86_CET_SET_LEGACY_BITMAP:
> + return handle_bitmap(arg2);
AFAICT it does exactly the opposite of that comment; it gets the address
and size from userspace and doesn't allocate anything at all.
^ permalink raw reply
* Re: [PATCH v7 22/27] binfmt_elf: Extract .note.gnu.property from an ELF file
From: Peter Zijlstra @ 2019-06-07 7:58 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200646.3951-23-yu-cheng.yu@intel.com>
On Thu, Jun 06, 2019 at 01:06:41PM -0700, Yu-cheng Yu wrote:
> An ELF file's .note.gnu.property indicates features the executable file
> can support. For example, the property GNU_PROPERTY_X86_FEATURE_1_AND
> indicates the file supports GNU_PROPERTY_X86_FEATURE_1_IBT and/or
> GNU_PROPERTY_X86_FEATURE_1_SHSTK.
>
> With this patch, if an arch needs to setup features from ELF properties,
> it needs CONFIG_ARCH_USE_GNU_PROPERTY to be set, and a specific
> arch_setup_property().
>
> For example, for X86_64:
>
> int arch_setup_property(void *ehdr, void *phdr, struct file *f, bool inter)
> {
> int r;
> uint32_t property;
>
> r = get_gnu_property(ehdr, phdr, f, GNU_PROPERTY_X86_FEATURE_1_AND,
> &property);
> ...
> }
>
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Did HJ write this patch as suggested by that SoB chain? If so, you lost
a From: line on top, if not, the SoB thing is invalid.
^ permalink raw reply
* Re: [PATCH v7 23/27] x86/cet/shstk: ELF header parsing of Shadow Stack
From: Peter Zijlstra @ 2019-06-07 7:54 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200646.3951-24-yu-cheng.yu@intel.com>
On Thu, Jun 06, 2019 at 01:06:42PM -0700, Yu-cheng Yu wrote:
> +#ifdef CONFIG_ARCH_USE_GNU_PROPERTY
> +int arch_setup_property(void *ehdr, void *phdr, struct file *f, bool inter)
> +{
> + int r;
> + uint32_t property;
Flip those two lines around.
> +
> + r = get_gnu_property(ehdr, phdr, f, GNU_PROPERTY_X86_FEATURE_1_AND,
> + &property);
> +
> + memset(¤t->thread.cet, 0, sizeof(struct cet_status));
It seems to me that memset would be better placed before
get_gnu_property().
> + if (r)
> + return r;
> +
> + if (cpu_feature_enabled(X86_FEATURE_SHSTK)) {
if (r || !cpu_feature_enabled())
return r;
> + if (property & GNU_PROPERTY_X86_FEATURE_1_SHSTK)
> + r = cet_setup_shstk();
> + if (r < 0)
> + return r;
> + }
> + return r;
and loose the indent.
> +}
> +#endif
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH] Documentation: DMA-API: fix a function name of max_mapping_size
From: Yoshihiro Shimoda @ 2019-06-07 7:47 UTC (permalink / raw)
To: corbet; +Cc: jroedel, hch, m.szyprowski, linux-doc, iommu, Yoshihiro Shimoda
The exported function name is dma_max_mapping_size(), not
dma_direct_max_mapping_size() so that this patch fixes
the function name in the documentation.
Fixes: 133d624b1cee ("dma: Introduce dma_max_mapping_size()")
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
Documentation/DMA-API.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 0076150..e47c63b 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -198,7 +198,7 @@ call to set the mask to the value returned.
::
size_t
- dma_direct_max_mapping_size(struct device *dev);
+ dma_max_mapping_size(struct device *dev);
Returns the maximum size of a mapping for the device. The size parameter
of the mapping functions like dma_map_single(), dma_map_page() and
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v7 18/27] mm: Introduce do_mmap_locked()
From: Peter Zijlstra @ 2019-06-07 7:47 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190607074322.GP3419@hirez.programming.kicks-ass.net>
On Fri, Jun 07, 2019 at 09:43:22AM +0200, Peter Zijlstra wrote:
> On Thu, Jun 06, 2019 at 01:06:37PM -0700, Yu-cheng Yu wrote:
> > There are a few places that need do_mmap() with mm->mmap_sem held.
> > Create an in-line function for that.
> >
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > ---
> > include/linux/mm.h | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 398f1e1c35e5..7cf014604848 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -2411,6 +2411,24 @@ static inline void mm_populate(unsigned long addr, unsigned long len)
> > static inline void mm_populate(unsigned long addr, unsigned long len) {}
> > #endif
> >
> > +static inline unsigned long do_mmap_locked(unsigned long addr,
> > + unsigned long len, unsigned long prot, unsigned long flags,
> > + vm_flags_t vm_flags)
> > +{
> > + struct mm_struct *mm = current->mm;
> > + unsigned long populate;
> > +
> > + down_write(&mm->mmap_sem);
> > + addr = do_mmap(NULL, addr, len, prot, flags, vm_flags, 0,
> > + &populate, NULL);
>
> Funny thing how do_mmap() takes a file pointer as first argument and
> this thing explicitly NULLs that. That more or less invalidates the name
> do_mmap_locked().
>
> > + up_write(&mm->mmap_sem);
> > +
> > + if (populate)
> > + mm_populate(addr, populate);
> > +
> > + return addr;
> > +}
You also don't retain that last @uf argument.
I'm thikning you're better off adding a helper to the cet.c file; call
it cet_mmap() or whatever.
^ permalink raw reply
* Re: [PATCH v7 18/27] mm: Introduce do_mmap_locked()
From: Peter Zijlstra @ 2019-06-07 7:43 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200646.3951-19-yu-cheng.yu@intel.com>
On Thu, Jun 06, 2019 at 01:06:37PM -0700, Yu-cheng Yu wrote:
> There are a few places that need do_mmap() with mm->mmap_sem held.
> Create an in-line function for that.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> include/linux/mm.h | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 398f1e1c35e5..7cf014604848 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2411,6 +2411,24 @@ static inline void mm_populate(unsigned long addr, unsigned long len)
> static inline void mm_populate(unsigned long addr, unsigned long len) {}
> #endif
>
> +static inline unsigned long do_mmap_locked(unsigned long addr,
> + unsigned long len, unsigned long prot, unsigned long flags,
> + vm_flags_t vm_flags)
> +{
> + struct mm_struct *mm = current->mm;
> + unsigned long populate;
> +
> + down_write(&mm->mmap_sem);
> + addr = do_mmap(NULL, addr, len, prot, flags, vm_flags, 0,
> + &populate, NULL);
Funny thing how do_mmap() takes a file pointer as first argument and
this thing explicitly NULLs that. That more or less invalidates the name
do_mmap_locked().
> + up_write(&mm->mmap_sem);
> +
> + if (populate)
> + mm_populate(addr, populate);
> +
> + return addr;
> +}
^ permalink raw reply
* Re: [PATCH v7 15/27] mm: Handle shadow stack page fault
From: Peter Zijlstra @ 2019-06-07 7:30 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200646.3951-16-yu-cheng.yu@intel.com>
On Thu, Jun 06, 2019 at 01:06:34PM -0700, Yu-cheng Yu wrote:
> diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
> index 75d9d68a6de7..ffcc0be7cadc 100644
> --- a/include/asm-generic/pgtable.h
> +++ b/include/asm-generic/pgtable.h
> @@ -1188,4 +1188,12 @@ static inline bool arch_has_pfn_modify_check(void)
> #define mm_pmd_folded(mm) __is_defined(__PAGETABLE_PMD_FOLDED)
> #endif
>
> +#ifndef CONFIG_ARCH_HAS_SHSTK
> +#define pte_set_vma_features(pte, vma) pte
> +#define arch_copy_pte_mapping(vma_flags) false
static inline pte_t pte_set_vma_features(pte_t pte, struct vm_area_struct *vma)
{
return pte;
}
static inline bool arch_copy_pte_mapping(unsigned long vm_flags)
{
return false;
}
Please, this way we retain function prototype checking.
> +#else
> +pte_t pte_set_vma_features(pte_t pte, struct vm_area_struct *vma);
> +bool arch_copy_pte_mapping(vm_flags_t vm_flags);
> +#endif
^ permalink raw reply
* Thanks and I wait for your answer
From: Martins Henry @ 2019-06-07 6:31 UTC (permalink / raw)
Hello,
I am Martin Henry, An American Citizen; I am the personal secretary to
Mr. Donald Railton, the controller of a Lottery Company. Please I am
having big problem now, I have a 6yrs old daughter who has leukemia, a
disease of the blood, and she needs a bone marrow transplant or she
will die.
Please I am only asking for your help and you will benefit from it
also. As an insider with Lottery Firm, working as the personal
secretary to the controller, I want you to send me your name to play,
I have some numbers that are going to win, stored in his secret data
system in the office. The Lottery is an online entry with credit card
anywhere with a name and address. All I want you to do is to send your
name to play it and I will send confirmation to you.
I will play with my card on your name and the Prize will be shared
equally between us. Immediately the results are released they will
contact you for payment as the oversea winner. The lotto can be played
with 9.00 dollars, or 50 dollars but the prize will be Millions.
Remember that I am playing on your name with my card; I just want to
front you for this, because I need this money to save the life of my
little daughter.
Thanks and I wait for your answer
Martin Henry.
^ permalink raw reply
* Re: [PATCH v7 05/27] x86/fpu/xstate: Add XSAVES system states for shadow stack
From: Peter Zijlstra @ 2019-06-07 7:07 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Randy Dunlap, Ravi V. Shankar,
Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200646.3951-6-yu-cheng.yu@intel.com>
On Thu, Jun 06, 2019 at 01:06:24PM -0700, Yu-cheng Yu wrote:
> Intel Control-flow Enforcement Technology (CET) introduces the
> following MSRs.
>
> MSR_IA32_U_CET (user-mode CET settings),
> MSR_IA32_PL3_SSP (user-mode shadow stack),
> MSR_IA32_PL0_SSP (kernel-mode shadow stack),
> MSR_IA32_PL1_SSP (Privilege Level 1 shadow stack),
> MSR_IA32_PL2_SSP (Privilege Level 2 shadow stack).
>
> Introduce them into XSAVES system states.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> arch/x86/include/asm/fpu/types.h | 22 +++++++++++++++++++++
> arch/x86/include/asm/fpu/xstate.h | 4 +++-
> arch/x86/include/uapi/asm/processor-flags.h | 2 ++
> arch/x86/kernel/fpu/xstate.c | 10 ++++++++++
> 4 files changed, 37 insertions(+), 1 deletion(-)
And yet, no changes to msr-index.h !?
^ permalink raw reply
* [PATCH] linux: README: reduced README size by 1 byte by removing unnecessary space character
From: Alex @ 2019-06-07 5:59 UTC (permalink / raw)
To: paulmck; +Cc: linux-doc, linux-kernel, trivial, Alex, Aaron A Montoya
From: Aaron A Montoya <aaron.a.montoya@gmail.com>
On line 9 of the README there is an unnecessary extra space character,
after the period, that adds 1 byte of size to the file. By removing the
unnecessary space, Linux downloads will be 1 byte smaller and therefor be
faster to download and take up less space on a user's system, all while
correcting a sentence structure issue. This change is one of the few
optimizations with practically no downsides, besides taking time out
of the hardworking Linux maintainers day to implement the change.
Remove extra space after the period on line 9 of the README.
Commit 0619770a02a30834 ("Removed unnecessary space character from README")
Signed-off-by: Aaron A Montoya <aaron.a.montoya@gmail.com>
---
README | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/README b/README
index 669ac7c32292..0b05d700637f 100644
--- a/README
+++ b/README
@@ -6,7 +6,7 @@ be rendered in a number of formats, like HTML and PDF. Please read
Documentation/admin-guide/README.rst first.
In order to build the documentation, use ``make htmldocs`` or
-``make pdfdocs``. The formatted documentation can also be read online at:
+``make pdfdocs``. The formatted documentation can also be read online at:
https://www.kernel.org/doc/html/latest/
--
2.21.0.windows.1
^ permalink raw reply related
* Re: [PATCH v7 04/27] x86/fpu/xstate: Introduce XSAVES system states
From: Andy Lutomirski @ 2019-06-07 1:54 UTC (permalink / raw)
To: Dave Hansen
Cc: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <4effb749-0cdc-6a49-6352-7b2d4aa7d866@intel.com>
> On Jun 6, 2019, at 3:08 PM, Dave Hansen <dave.hansen@intel.com> wrote:
>
>
>
> On 6/6/19 3:04 PM, Andy Lutomirski wrote:
>>> But, that seems broken. If we have supervisor state, we can't
>>> always defer the load until return to userspace, so we'll never??
>>> have TIF_NEED_FPU_LOAD. That would certainly be true for
>>> cet_kernel_state.
>>
>> Ugh. I was sort of imagining that we would treat supervisor state
> completely separately from user state. But can you maybe give
> examples of exactly what you mean?
I was imagining a completely separate area in memory for supervisor states. I guess this might defeat the modified optimization and is probably a bad idea.
>>
>>> It seems like we actually need three classes of XSAVE states: 1.
>>> User state
>>
>> This is FPU, XMM, etc, right?
>
> Yep.
>
>>> 2. Supervisor state that affects user mode
>>
>> User CET?
>
> Yep.
>
>>> 3. Supervisor state that affects kernel mode
>>
>> Like supervisor CET? If we start doing supervisor shadow stack, the
>> context switches will be real fun. We may need to handle this in
>> asm.
>
> Yeah, that's what I was thinking.
>
> I have the feeling Yu-cheng's patches don't comprehend this since
> Sebastian's patches went in after he started working on shadow stacks.
Do we need to have TIF_LOAD_FPU mean “we need to load *some* of the xsave state”? If so, maybe a bunch of the accessors should have their interfaces reviewed to make sure they’re sill sensible.
>
>> Where does PKRU fit in? Maybe we can treat it as #3?
>
> I thought Sebastian added specific PKRU handling to make it always
> eager. It's actually user state that affect kernel mode. :)
Indeed. But, if we document a taxonomy of states, we should make sure it fits in. I guess it’s like supervisor CET except that user code can can also read and write it.
We should probably have self tests that make sure that the correct states, and nothing else, show up in ptrace and signal states, and that trying to write supervisor CET via ptrace and sigreturn is properly rejected.
Just to double check my mental model: it’s okay to XSAVES twice to the same buffer with disjoint RFBM as long as we do something intelligent with XSTATE_BV afterwards, right? Because, unless we split up the buffers, I think we will have to do this when we context switch while TIF_LOAD_FPU is set.
Are there performance numbers for how the time needed to XRSTORS everything versus the time to XRSTORS supervisor CET and then separately XRSTORS the FPU? This may affect whether we want context switches to have the new task eagerly or lazily restored.
Hmm. I wonder if we need some way for a selftest to reliably trigger TIF_LOAD_FPU.
—Andy
^ permalink raw reply
* [PATCH v4] RISC-V: Add an Image header that boot loader can parse.
From: Atish Patra @ 2019-06-06 23:08 UTC (permalink / raw)
To: linux-kernel
Cc: Atish Patra, Karsten Merker, Kevin Hilman, Albert Ou, Anup Patel,
Jonathan Corbet, linux-doc, linux-riscv, Nick Kossifidis,
Palmer Dabbelt, paul.walmsley@sifive.com,
linux-arm-kernel@lists.infradead.org, mark.rutland@arm.com,
marek.vasut@gmail.com, catalin.marinas@arm.com,
will.deacon@arm.com, trini@konsulko.com
Currently, the last stage boot loaders such as U-Boot can accept only
uImage which is an unnecessary additional step in automating boot
process.
Add an image header that boot loader understands and boot Linux from
flat Image directly.
This header is based on ARM64 boot image header and provides an
opportunity to combine both ARM64 & RISC-V image headers in future.
Also make sure that PE/COFF header can co-exist in the same image so
that EFI stub can be supported for RISC-V in future. EFI specification
needs PE/COFF image header in the beginning of the kernel image in order
to load it as an EFI application. In order to support EFI stub, code0
should be replaced with "MZ" magic string and res4(at offset 0x3c)
should point to the rest of the PE/COFF header (which will be added
during EFI support).
Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
Signed-off-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Karsten Merker <merker@debian.org>
Tested-by: Karsten Merker <merker@debian.org> (QEMU+OpenSBI+U-Boot)
Tested-by: Kevin Hilman <khilman@baylibre.com> (OpenSBI + U-Boot + Linux)
---
I have not sent out corresponding U-Boot patch as all the changes are
compatible with current u-boot support. Once, the kernel header format
is agreed upon, I will update the U-Boot patch.
Changes from v4->v5
1. Error if CONFIG_CPU_BIG_ENDIAN is enabled in kernel.
2. Typo fix
Changes from v3->v4
1. Update the commit text to clarify about PE/COFF header.
Changes from v2->v3
1. Modified reserved fields to define a header version.
2. Added header documentation.
Changes from v1-v2:
1. Added additional reserved elements to make it fully PE compatible.
---
| 50 ++++++++++++++++++
arch/riscv/include/asm/image.h | 62 +++++++++++++++++++++++
arch/riscv/kernel/head.S | 32 ++++++++++++
3 files changed, 144 insertions(+)
create mode 100644 Documentation/riscv/boot-image-header.txt
create mode 100644 arch/riscv/include/asm/image.h
--git a/Documentation/riscv/boot-image-header.txt b/Documentation/riscv/boot-image-header.txt
new file mode 100644
index 000000000000..acbf3b4cacfe
--- /dev/null
+++ b/Documentation/riscv/boot-image-header.txt
@@ -0,0 +1,50 @@
+ Boot image header in RISC-V Linux
+ =============================================
+
+Author: Atish Patra <atish.patra@wdc.com>
+Date : 20 May 2019
+
+This document only describes the boot image header details for RISC-V Linux.
+The complete booting guide will be available at Documentation/riscv/booting.txt.
+
+The following 64-byte header is present in decompressed Linux kernel image.
+
+ u32 code0; /* Executable code */
+ u32 code1; /* Executable code */
+ u64 text_offset; /* Image load offset, little endian */
+ u64 image_size; /* Effective Image size, little endian */
+ u64 flags; /* kernel flags, little endian */
+ u32 version; /* Version of this header */
+ u32 res1 = 0; /* Reserved */
+ u64 res2 = 0; /* Reserved */
+ u64 magic = 0x5643534952; /* Magic number, little endian, "RISCV" */
+ u32 res3; /* Reserved for additional RISC-V specific header */
+ u32 res4; /* Reserved for PE COFF offset */
+
+This header format is compliant with PE/COFF header and largely inspired from
+ARM64 header. Thus, both ARM64 & RISC-V header can be combined into one common
+header in future.
+
+Notes:
+- This header can also be reused to support EFI stub for RISC-V in future. EFI
+ specification needs PE/COFF image header in the beginning of the kernel image
+ in order to load it as an EFI application. In order to support EFI stub,
+ code0 should be replaced with "MZ" magic string and res5(at offset 0x3c) should
+ point to the rest of the PE/COFF header.
+
+- version field indicate header version number.
+ Bits 0:15 - Minor version
+ Bits 16:31 - Major version
+
+ This preserves compatibility across newer and older version of the header.
+ The current version is defined as 0.1.
+
+- res3 is reserved for offset to any other additional fields. This makes the
+ header extendible in future. One example would be to accommodate ISA
+ extension for RISC-V in future. For current version, it is set to be zero.
+
+- In current header, the flag field has only one field.
+ Bit 0: Kernel endianness. 1 if BE, 0 if LE.
+
+- Image size is mandatory for boot loader to load kernel image. Booting will
+ fail otherwise.
diff --git a/arch/riscv/include/asm/image.h b/arch/riscv/include/asm/image.h
new file mode 100644
index 000000000000..13f4365d2dd6
--- /dev/null
+++ b/arch/riscv/include/asm/image.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ASM_IMAGE_H
+#define __ASM_IMAGE_H
+
+#define RISCV_IMAGE_MAGIC "RISCV"
+
+#define RISCV_IMAGE_FLAG_BE_SHIFT 0
+#define RISCV_IMAGE_FLAG_BE_MASK 0x1
+
+#define RISCV_IMAGE_FLAG_LE 0
+#define RISCV_IMAGE_FLAG_BE 1
+
+#ifdef CONFIG_CPU_BIG_ENDIAN
+#error conversion of header fields to LE not yet implemented
+#else
+#define __HEAD_FLAG_BE RISCV_IMAGE_FLAG_LE
+#endif
+
+#define __HEAD_FLAG(field) (__HEAD_FLAG_##field << \
+ RISCV_IMAGE_FLAG_##field##_SHIFT)
+
+#define __HEAD_FLAGS (__HEAD_FLAG(BE))
+
+#define RISCV_HEADER_VERSION_MAJOR 0
+#define RISCV_HEADER_VERSION_MINOR 1
+
+#define RISCV_HEADER_VERSION (RISCV_HEADER_VERSION_MAJOR << 16 | \
+ RISCV_HEADER_VERSION_MINOR)
+
+#ifndef __ASSEMBLY__
+/*
+ * struct riscv_image_header - riscv kernel image header
+ *
+ * @code0: Executable code
+ * @code1: Executable code
+ * @text_offset: Image load offset
+ * @image_size: Effective Image size
+ * @flags: kernel flags
+ * @version: version
+ * @reserved: reserved
+ * @reserved: reserved
+ * @magic: Magic number
+ * @reserved: reserved (will be used for additional RISC-V specific header)
+ * @reserved: reserved (will be used for PE COFF offset)
+ */
+
+struct riscv_image_header {
+ u32 code0;
+ u32 code1;
+ u64 text_offset;
+ u64 image_size;
+ u64 flags;
+ u32 version;
+ u32 res1;
+ u64 res2;
+ u64 magic;
+ u32 res3;
+ u32 res4;
+};
+#endif /* __ASSEMBLY__ */
+#endif /* __ASM_IMAGE_H */
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 370c66ce187a..577893bb150d 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -19,9 +19,41 @@
#include <asm/thread_info.h>
#include <asm/page.h>
#include <asm/csr.h>
+#include <asm/image.h>
__INIT
ENTRY(_start)
+ /*
+ * Image header expected by Linux boot-loaders. The image header data
+ * structure is described in asm/image.h.
+ * Do not modify it without modifying the structure and all bootloaders
+ * that expects this header format!!
+ */
+ /* jump to start kernel */
+ j _start_kernel
+ /* reserved */
+ .word 0
+ .balign 8
+#if __riscv_xlen == 64
+ /* Image load offset(2MB) from start of RAM */
+ .dword 0x200000
+#else
+ /* Image load offset(4MB) from start of RAM */
+ .dword 0x400000
+#endif
+ /* Effective size of kernel image */
+ .dword _end - _start
+ .dword __HEAD_FLAGS
+ .word RISCV_HEADER_VERSION
+ .word 0
+ .dword 0
+ .asciz RISCV_IMAGE_MAGIC
+ .word 0
+ .balign 4
+ .word 0
+
+.global _start_kernel
+_start_kernel:
/* Mask all interrupts */
csrw CSR_SIE, zero
csrw CSR_SIP, zero
--
2.21.0
^ permalink raw reply related
* Re: [v3 PATCH] RISC-V: Add a PE/COFF compliant Image header.
From: Atish Patra @ 2019-06-06 22:41 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-kernel@vger.kernel.org, Albert Ou, Anup Patel,
Jonathan Corbet, linux-doc@vger.kernel.org,
linux-riscv@lists.infradead.org, Nick Kossifidis, Palmer Dabbelt,
Zong Li, linux-arm-kernel@lists.infradead.org,
marek.vasut@gmail.com, catalin.marinas@arm.com,
will.deacon@arm.com, trini@konsulko.com, paul.walmsley@sifive.com
In-Reply-To: <20190605162630.GE30925@lakrids.cambridge.arm.com>
On 6/5/19 9:26 AM, Mark Rutland wrote:
> On Thu, May 23, 2019 at 11:35:16AM -0700, Atish Patra wrote:
>> Currently, last stage boot loaders such as U-Boot can accept only
>> uImage which is an unnecessary additional step in automating boot flows.
>>
>> Add a PE/COFF compliant image header that boot loaders can parse and
>> directly load kernel flat Image. The existing booting methods will continue
>> to work as it is.
>>
>> Another goal of this header is to support EFI stub for RISC-V in future.
>> EFI specification needs PE/COFF image header in the beginning of the kernel
>> image in order to load it as an EFI application. In order to support
>> EFI stub, code0 should be replaced with "MZ" magic string and res5(at
>> offset 0x3c) should point to the rest of the PE/COFF header (which will
>> be added during EFI support).
>>
>> This patch is based on ARM64 boot image header and provides an opprtunity
>> to combine both ARM64 & RISC-V image headers.
>>
>> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
>>
>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>>
>> ---
>> I have not sent out corresponding U-Boot patch as all the changes are
>> compatible with current u-boot support. Once, the kernel header format
>> is agreed upon, I will update the U-Boot patch.
>>
>> Changes from v2->v3
>> 1. Modified reserved fields to define a header version.
>> 2. Added header documentation.
>>
>> Changes from v1-v2:
>> 1. Added additional reserved elements to make it fully PE compatible.
>> ---
>> Documentation/riscv/boot-image-header.txt | 50 ++++++++++++++++++
>> arch/riscv/include/asm/image.h | 64 +++++++++++++++++++++++
>> arch/riscv/kernel/head.S | 32 ++++++++++++
>> 3 files changed, 146 insertions(+)
>> create mode 100644 Documentation/riscv/boot-image-header.txt
>> create mode 100644 arch/riscv/include/asm/image.h
>>
>> diff --git a/Documentation/riscv/boot-image-header.txt b/Documentation/riscv/boot-image-header.txt
>> new file mode 100644
>> index 000000000000..68abc2353cec
>> --- /dev/null
>> +++ b/Documentation/riscv/boot-image-header.txt
>> @@ -0,0 +1,50 @@
>> + Boot image header in RISC-V Linux
>> + =============================================
>> +
>> +Author: Atish Patra <atish.patra@wdc.com>
>> +Date : 20 May 2019
>> +
>> +This document only describes the boot image header details for RISC-V Linux.
>> +The complete booting guide will be available at Documentation/riscv/booting.txt.
>> +
>> +The following 64-byte header is present in decompressed Linux kernel image.
>> +
>> + u32 code0; /* Executable code */
>> + u32 code1; /* Executable code */
>> + u64 text_offset; /* Image load offset, little endian */
>> + u64 image_size; /* Effective Image size, little endian */
>> + u64 flags; /* kernel flags, little endian */
>> + u32 version; /* Version of this header */
>> + u32 res1 = 0; /* Reserved */
>> + u64 res2 = 0; /* Reserved */
>> + u64 magic = 0x5643534952; /* Magic number, little endian, "RISCV" */
>> + u32 res3; /* Reserved for additional RISC-V specific header */
>> + u32 res4; /* Reserved for PE COFF offset */
>> +
>> +This header format is compliant with PE/COFF header and largely inspired from
>> +ARM64 header. Thus, both ARM64 & RISC-V header can be combined into one common
>> +header in future.
>> +
>> +Notes:
>> +- This header can also be reused to support EFI stub for RISC-V in future. EFI
>> + specification needs PE/COFF image header in the beginning of the kernel image
>> + in order to load it as an EFI application. In order to support EFI stub,
>> + code0 should be replaced with "MZ" magic string and res5(at offset 0x3c) should
>> + point to the rest of the PE/COFF header.
>> +
>> +- version field indicate header version number.
>> + Bits 0:15 - Minor version
>> + Bits 16:31 - Major version
>> +
>> + This preserves compatibility across newer and older version of the header.
>> + The current version is defined as 0.1.
>> +
>> +- res3 is reserved for offset to any other additional fields. This makes the
>> + header extendible in future. One example would be to accommodate ISA
>> + extension for RISC-V in future. For current version, it is set to be zero.
>> +
>> +- In current header, the flag field has only one field.
>> + Bit 0: Kernel endianness. 1 if BE, 0 if LE.
>> +
>> +- Image size is mandatory for boot loader to load kernel image. Booting will
>> + fail otherwise.
>> diff --git a/arch/riscv/include/asm/image.h b/arch/riscv/include/asm/image.h
>> new file mode 100644
>> index 000000000000..61c9f20d2f19
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/image.h
>> @@ -0,0 +1,64 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef __ASM_IMAGE_H
>> +#define __ASM_IMAGE_H
>> +
>> +#define RISCV_IMAGE_MAGIC "RISCV"
>> +
>> +
>> +#define RISCV_IMAGE_FLAG_BE_SHIFT 0
>> +#define RISCV_IMAGE_FLAG_BE_MASK 0x1
>> +
>> +#define RISCV_IMAGE_FLAG_LE 0
>> +#define RISCV_IMAGE_FLAG_BE 1
>> +
>> +
>> +#ifdef CONFIG_CPU_BIG_ENDIAN
>> +#define __HEAD_FLAG_BE RISCV_IMAGE_FLAG_BE
>> +#else
>> +#define __HEAD_FLAG_BE RISCV_IMAGE_FLAG_LE
>> +#endif
>> +
>> +#define __HEAD_FLAG(field) (__HEAD_FLAG_##field << \
>> + RISCV_IMAGE_FLAG_##field##_SHIFT)
>> +
>> +#define __HEAD_FLAGS (__HEAD_FLAG(BE))
>
> If you have a CONFIG_CPU_BIG_ENDIAN kernel, this will not be
> little-endian, nor will other fields in your header (e.g. the image
> size), so I would recommend dropping this for now.
>
Correct. Thanks for pointing that out.
> To manage that for the image_size field you'll probably need to play the
> same linker trick games we play on arm64.
>
> It's probably worth having:
>
> #ifdef CONFIG_CPU_BIG_ENDIAN
> #error conversion of header fields to LE not yet implemented
> #endif
>
Sure. I will update the patch.
Regards,
Atish
> ... to catch that later.
>
> Thanks,
> Mark,
>
--
Regards,
Atish
^ permalink raw reply
* Re: [PATCH v7 04/27] x86/fpu/xstate: Introduce XSAVES system states
From: Yu-cheng Yu @ 2019-06-06 22:10 UTC (permalink / raw)
To: Dave Hansen, Andy Lutomirski
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Balbir Singh, Borislav Petkov, Cyrill Gorcunov, Dave Hansen,
Eugene Syromiatnikov, Florian Weimer, H.J. Lu, Jann Horn,
Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <4effb749-0cdc-6a49-6352-7b2d4aa7d866@intel.com>
On Thu, 2019-06-06 at 15:08 -0700, Dave Hansen wrote:
>
> On 6/6/19 3:04 PM, Andy Lutomirski wrote:
> > > But, that seems broken. If we have supervisor state, we can't
> > > always defer the load until return to userspace, so we'll never??
> > > have TIF_NEED_FPU_LOAD. That would certainly be true for
> > > cet_kernel_state.
> >
> > Ugh. I was sort of imagining that we would treat supervisor state
>
> completely separately from user state. But can you maybe give
> examples of exactly what you mean?
> >
> > > It seems like we actually need three classes of XSAVE states: 1.
> > > User state
> >
> > This is FPU, XMM, etc, right?
>
> Yep.
>
> > > 2. Supervisor state that affects user mode
> >
> > User CET?
>
> Yep.
>
> > > 3. Supervisor state that affects kernel mode
> >
> > Like supervisor CET? If we start doing supervisor shadow stack, the
> > context switches will be real fun. We may need to handle this in
> > asm.
>
> Yeah, that's what I was thinking.
>
> I have the feeling Yu-cheng's patches don't comprehend this since
> Sebastian's patches went in after he started working on shadow stacks.
>
> > Where does PKRU fit in? Maybe we can treat it as #3?
>
> I thought Sebastian added specific PKRU handling to make it always
> eager. It's actually user state that affect kernel mode. :)
For CET user states, we need to restore before making changes. If they are not
being changed (i.e. signal handling and syscalls), then they are restored only
before going back to user-mode.
For CET kernel states, we only need to make small changes in the way similar to
the PKRU handling, right? We'll address it when sending CET kernel-mode
patches.
I will put in more comments as suggested by Dave in earlier emails.
Yu-cheng
^ permalink raw reply
* Re: [PATCH v7 04/27] x86/fpu/xstate: Introduce XSAVES system states
From: Dave Hansen @ 2019-06-06 22:08 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <5EE146A8-6C8C-4C5D-B7C0-AB8AD1012F1E@amacapital.net>
On 6/6/19 3:04 PM, Andy Lutomirski wrote:
>> But, that seems broken. If we have supervisor state, we can't
>> always defer the load until return to userspace, so we'll never??
>> have TIF_NEED_FPU_LOAD. That would certainly be true for
>> cet_kernel_state.
>
> Ugh. I was sort of imagining that we would treat supervisor state
completely separately from user state. But can you maybe give
examples of exactly what you mean?
>
>> It seems like we actually need three classes of XSAVE states: 1.
>> User state
>
> This is FPU, XMM, etc, right?
Yep.
>> 2. Supervisor state that affects user mode
>
> User CET?
Yep.
>> 3. Supervisor state that affects kernel mode
>
> Like supervisor CET? If we start doing supervisor shadow stack, the
> context switches will be real fun. We may need to handle this in
> asm.
Yeah, that's what I was thinking.
I have the feeling Yu-cheng's patches don't comprehend this since
Sebastian's patches went in after he started working on shadow stacks.
> Where does PKRU fit in? Maybe we can treat it as #3?
I thought Sebastian added specific PKRU handling to make it always
eager. It's actually user state that affect kernel mode. :)
^ permalink raw reply
* Re: [PATCH v7 04/27] x86/fpu/xstate: Introduce XSAVES system states
From: Andy Lutomirski @ 2019-06-06 22:04 UTC (permalink / raw)
To: Dave Hansen
Cc: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <0a2f8b9b-b96b-06c8-bae0-b78b2ca3b727@intel.com>
On Jun 6, 2019, at 2:18 PM, Dave Hansen <dave.hansen@intel.com> wrote:
>> +/*
>> + * Helpers for changing XSAVES system states.
>> + */
>> +static inline void modify_fpu_regs_begin(void)
>> +{
>> + fpregs_lock();
>> + if (test_thread_flag(TIF_NEED_FPU_LOAD))
>> + __fpregs_load_activate();
>> +}
>> +
>> +static inline void modify_fpu_regs_end(void)
>> +{
>> + fpregs_unlock();
>> +}
>
> These are massively under-commented and under-changelogged. This looks
> like it's intended to ensure that we have supervisor FPU state for this
> task loaded before we go and run the MSRs that might be modifying it.
>
> But, that seems broken. If we have supervisor state, we can't always
> defer the load until return to userspace, so we'll never?? have
> TIF_NEED_FPU_LOAD. That would certainly be true for cet_kernel_state.
Ugh. I was sort of imagining that we would treat supervisor state completely separately from user state. But can you maybe give examples of exactly what you mean?
>
> It seems like we actually need three classes of XSAVE states:
> 1. User state
This is FPU, XMM, etc, right?
> 2. Supervisor state that affects user mode
User CET?
> 3. Supervisor state that affects kernel mode
Like supervisor CET? If we start doing supervisor shadow stack, the context switches will be real fun. We may need to handle this in asm.
Where does PKRU fit in? Maybe we can treat it as #3?
—Andy
^ permalink raw reply
* Re: [PATCH v7 04/27] x86/fpu/xstate: Introduce XSAVES system states
From: Dave Hansen @ 2019-06-06 21:18 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Borislav Petkov,
Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra, Randy Dunlap, Ravi V. Shankar, Vedvyas Shanbhogue,
Dave Martin
In-Reply-To: <20190606200646.3951-5-yu-cheng.yu@intel.com>
> +/*
> + * Helpers for changing XSAVES system states.
> + */
> +static inline void modify_fpu_regs_begin(void)
> +{
> + fpregs_lock();
> + if (test_thread_flag(TIF_NEED_FPU_LOAD))
> + __fpregs_load_activate();
> +}
> +
> +static inline void modify_fpu_regs_end(void)
> +{
> + fpregs_unlock();
> +}
These are massively under-commented and under-changelogged. This looks
like it's intended to ensure that we have supervisor FPU state for this
task loaded before we go and run the MSRs that might be modifying it.
But, that seems broken. If we have supervisor state, we can't always
defer the load until return to userspace, so we'll never?? have
TIF_NEED_FPU_LOAD. That would certainly be true for cet_kernel_state.
It seems like we actually need three classes of XSAVE states:
1. User state
2. Supervisor state that affects user mode
3. Supervisor state that affects kernel mode
We can delay the load of 1 and 2, but not 3.
But I don't see any infrastructure for this.
^ permalink raw reply
* Re: [PATCH 03/10] mfd / platform: cros_ec: Miscellaneous character device to talk with the EC
From: Randy Dunlap @ 2019-06-06 21:11 UTC (permalink / raw)
To: Ezequiel Garcia, Greg Kroah-Hartman
Cc: Guenter Roeck, Enric Balletbo i Serra, linux-kernel,
Gwendal Grignou, Guenter Roeck, Benson Leung, Lee Jones, kernel,
Dmitry Torokhov, Gustavo Pimentel, Lorenzo Pieralisi, linux-doc,
Enno Luebbers, Guido Kiener, Thomas Gleixner,
Kishon Vijay Abraham I, Jonathan Corbet, Wu Hao, Kate Stewart,
Tycho Andersen, Gerd Hoffmann, Jilayne Lovejoy
In-Reply-To: <1cfc4bfab8d9d8a47e5dacaca88a7fe30ae83076.camel@collabora.com>
On 6/6/19 8:12 AM, Ezequiel Garcia wrote:
> On Thu, 2019-06-06 at 16:51 +0200, Greg Kroah-Hartman wrote:
>> On Thu, Jun 06, 2019 at 11:01:17AM -0300, Ezequiel Garcia wrote:
>>> On Tue, 2019-06-04 at 20:59 +0200, Greg Kroah-Hartman wrote:
>>>> On Tue, Jun 04, 2019 at 11:39:21AM -0700, Guenter Roeck wrote:
>>>>> On Tue, Jun 4, 2019 at 11:35 AM Greg Kroah-Hartman
>>>>> <gregkh@linuxfoundation.org> wrote:
>>>>>> On Tue, Jun 04, 2019 at 01:58:38PM -0300, Ezequiel Garcia wrote:
>>>>>>> Hey Greg,
>>>>>>>
>>>>>>>>> + dev_info(&pdev->dev, "Created misc device /dev/%s\n",
>>>>>>>>> + data->misc.name);
>>>>>>>>
>>>>>>>> No need to be noisy, if all goes well, your code should be quiet.
>>>>>>>>
>>>>>>>
>>>>>>> I sometimes wonder about this being noise or not, so I will slightly
>>>>>>> hijack this thread for this discussion.
>>>>>>>
>>>>>>>> From a kernel developer point-of-view, or even from a platform
>>>>>>> developer or user with a debugging hat point-of-view, having
>>>>>>> a "device created" or "device registered" message is often very useful.
>>>>>>
>>>>>> For you, yes. For someone with 30000 devices attached to their system,
>>>>>> it is not, and causes booting to take longer than it should be.
>>>>>>
>>>>>>> In fact, I wish people would do this more often, so I don't have to
>>>>>>> deal with dynamic debug, or hack my way:
>>>>>>>
>>>>>>> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
>>>>>>> index 4589631798c9..473549b26bb2 100644
>>>>>>> --- a/drivers/media/i2c/ov5647.c
>>>>>>> +++ b/drivers/media/i2c/ov5647.c
>>>>>>> @@ -603,7 +603,7 @@ static int ov5647_probe(struct i2c_client *client,
>>>>>>> if (ret < 0)
>>>>>>> goto error;
>>>>>>>
>>>>>>> - dev_dbg(dev, "OmniVision OV5647 camera driver probed\n");
>>>>>>> + dev_info(dev, "OmniVision OV5647 camera driver probed\n");
>>>>>>> return 0;
>>>>>>> error:
>>>>>>> media_entity_cleanup(&sd->entity);
>>>>>>>
>>>>>>> In some subsystems, it's even a behavior I'm more or less relying on:
>>>>>>>
>>>>>>> $ git grep v4l2_info.*registered drivers/media/ | wc -l
>>>>>>> 26
>>>>>>>
>>>>>>> And on the downsides, I can't find much. It's just one little line,
>>>>>>> that is not even noticed unless you have logging turned on.
>>>>>>
>>>>>> Its better to be quiet, which is why the "default driver registration"
>>>>>> macros do not have any printk messages in them. When converting drivers
>>>>>> over to it, we made the boot process much more sane, don't try to go and
>>>>>> add messages for no good reason back in please.
>>>>>>
>>>>>> dynamic debugging can be enabled on a module and line-by-line basis,
>>>>>> even from the boot command line. So if you need debugging, you can
>>>>>> always ask someone to just reboot or unload/load the module and get the
>>>>>> message that way.
>>>>>>
>>>>>
>>>>> Can we by any chance make this an official policy ? I am kind of tired
>>>>> having to argue about this over and over again.
>>>>
>>>> Sure, but how does anyone make any "official policy" in the kernel? :)
>>>>
>>>> I could just go through and delete all "look ma, a new driver/device!"
>>>> messages, but that might be annoying...
>>>>
>>>
>>> Well, I really need to task.
>>
>> ???
>>
>
> Oops, typo: s/task/ask :-)
>
>>> If it's not an official policy (and won't be anytime soon?),
>>
>> The ":)" there was that we really have very few "official" policies,
>> only things that we all strongly encourage to happen. And get grumpy if
>> we see them in code reviews. Like I did here.
>>
>
> Well, not everyone gets grumpy. As I pointed out, we use this "registered"
> messages (messages or noise, seems this lie in the eye of the beholder),
> consistently across entire subsystems.
:(
>>> then what's preventing Enric from pushing this print on this driver,
>>> given he is the one maintaining the code?
>>
>> Given that he wants people to review his code, why would you tell him to
>> ignore what people are trying to tell him?
>>
>
> I'm not suggesting to ignore anyone, rather to consider all voices
> involved in each review comment.
>
>> Again, don't be noisy, it's not hard, and is how things have been
>> trending for many years now.
Ack that.
--
~Randy
^ permalink raw reply
* Re: [PATCH v7 11/14] x86/vsyscall/64: Add ENDBR64 to vsyscall entry points
From: Andy Lutomirski @ 2019-06-06 20:28 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200926.4029-12-yu-cheng.yu@intel.com>
On Thu, Jun 6, 2019 at 1:17 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> From: "H.J. Lu" <hjl.tools@gmail.com>
>
> Add ENDBR64 to vsyscall entry points.
I'm still okay with this patch, but this is rather silly. If anyone
actually executes this code, they're doing it wrong.
--Andy
^ permalink raw reply
* Re: [PATCH v7 12/14] x86/vsyscall/64: Fixup shadow stack and branch tracking for vsyscall
From: Andy Lutomirski @ 2019-06-06 20:27 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
open list:DOCUMENTATION, Linux-MM, linux-arch, Linux API,
Arnd Bergmann, Balbir Singh, Borislav Petkov, Cyrill Gorcunov,
Dave Hansen, Eugene Syromiatnikov, Florian Weimer, H.J. Lu,
Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190606200926.4029-13-yu-cheng.yu@intel.com>
On Thu, Jun 6, 2019 at 1:17 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> When emulating a RET, also unwind the task's shadow stack and cancel
> the current branch tracking status.
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> arch/x86/entry/vsyscall/vsyscall_64.c | 28 +++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
> index d9d81ad7a400..6869ef9d1e8b 100644
> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> @@ -38,6 +38,8 @@
> #include <asm/fixmap.h>
> #include <asm/traps.h>
> #include <asm/paravirt.h>
> +#include <asm/fpu/xstate.h>
> +#include <asm/fpu/types.h>
>
> #define CREATE_TRACE_POINTS
> #include "vsyscall_trace.h"
> @@ -92,6 +94,30 @@ static int addr_to_vsyscall_nr(unsigned long addr)
> return nr;
> }
>
> +void fixup_shstk(void)
> +{
> +#ifdef CONFIG_X86_INTEL_SHADOW_STACK_USER
> + u64 r;
> +
> + if (current->thread.cet.shstk_enabled) {
> + rdmsrl(MSR_IA32_PL3_SSP, r);
> + wrmsrl(MSR_IA32_PL3_SSP, r + 8);
> + }
> +#endif
> +}
> +
> +void fixup_ibt(void)
> +{
> +#ifdef CONFIG_X86_INTEL_BRANCH_TRACKING_USER
> + u64 r;
> +
> + if (current->thread.cet.ibt_enabled) {
> + rdmsrl(MSR_IA32_U_CET, r);
> + wrmsrl(MSR_IA32_U_CET, r & ~MSR_IA32_CET_WAIT_ENDBR);
> + }
> +#endif
> +}
These should be static.
But please just inline them directly in their one call site. The code
will be a lot easier to understand.
--Andy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox