* Re: [PATCH] powerpc: enable a 30-bit ZONE_DMA for 32-bit pmac
From: Aaro Koskinen @ 2019-06-14 19:15 UTC (permalink / raw)
To: Mathieu Malaterre
Cc: LKML, Paul Mackerras, linuxppc-dev, Christoph Hellwig,
Larry Finger
In-Reply-To: <CA+7wUswMtpVCoX0H5eF=GUY8jWDAEWa9Z223tKiKHiL69hhHtQ@mail.gmail.com>
Hi,
On Fri, Jun 14, 2019 at 09:24:16AM +0200, Mathieu Malaterre wrote:
> On Thu, Jun 13, 2019 at 10:27 AM Christoph Hellwig <hch@lst.de> wrote:
> > With the strict dma mask checking introduced with the switch to
> > the generic DMA direct code common wifi chips on 32-bit powerbooks
> > stopped working. Add a 30-bit ZONE_DMA to the 32-bit pmac builds
> > to allow them to reliably allocate dma coherent memory.
> >
> > Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
> > Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> > arch/powerpc/include/asm/page.h | 7 +++++++
> > arch/powerpc/mm/mem.c | 3 ++-
> > arch/powerpc/platforms/powermac/Kconfig | 1 +
> > 3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> > index b8286a2013b4..0d52f57fca04 100644
> > --- a/arch/powerpc/include/asm/page.h
> > +++ b/arch/powerpc/include/asm/page.h
> > @@ -319,6 +319,13 @@ struct vm_area_struct;
> > #endif /* __ASSEMBLY__ */
> > #include <asm/slice.h>
> >
> > +/*
> > + * Allow 30-bit DMA for very limited Broadcom wifi chips on many powerbooks.
>
> nit: would it be possible to mention explicit reference to b43-legacy.
> Using b43 on my macmini g4 never showed those symptoms (using
> 5.2.0-rc2+)
According to Wikipedia Mac mini G4 is limited to 1 GB RAM, so that's
why you don't see the issue.
A.
^ permalink raw reply
* Re: [PATCH v1 1/6] mm: Section numbers use the type "unsigned long"
From: David Hildenbrand @ 2019-06-14 19:34 UTC (permalink / raw)
To: Andrew Morton
Cc: Stephen Rothwell, Michal Hocko, Mel Gorman, Baoquan He, linux-mm,
Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel, Wei Yang,
linux-acpi, Mike Rapoport, Arun KS, Johannes Weiner,
Pavel Tatashin, Dan Williams, linuxppc-dev, Vlastimil Babka,
Oscar Salvador
In-Reply-To: <20190614120036.00ae392e3f210e7bc9ec6960@linux-foundation.org>
On 14.06.19 21:00, Andrew Morton wrote:
> On Fri, 14 Jun 2019 12:01:09 +0200 David Hildenbrand <david@redhat.com> wrote:
>
>> We are using a mixture of "int" and "unsigned long". Let's make this
>> consistent by using "unsigned long" everywhere. We'll do the same with
>> memory block ids next.
>>
>> ...
>>
>> - int i, ret, section_count = 0;
>> + unsigned long i;
>>
>> ...
>>
>> - unsigned int i;
>> + unsigned long i;
>
> Maybe I did too much fortran back in the day, but I think the
> expectation is that a variable called "i" has type "int".
>
> This?
t460s: ~/git/linux memory_block_devices2 $ git grep "unsigned long i;" |
wc -l
245
t460s: ~/git/linux memory_block_devices2 $ git grep "int i;" | wc -l
26827
Yes ;)
While it makes sense for the second and third occurrence, I think for
the first one it could be confusing (it's not actually a section number
but a counter for sections_per_block).
I see just now that we can avoid converting the first occurrence
completely. So maybe we should drop changing removable_show() from this
patch.
Cheers!
>
>
>
> s/unsigned long i/unsigned long section_nr/
>
> --- a/drivers/base/memory.c~mm-section-numbers-use-the-type-unsigned-long-fix
> +++ a/drivers/base/memory.c
> @@ -131,17 +131,17 @@ static ssize_t phys_index_show(struct de
> static ssize_t removable_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - unsigned long i, pfn;
> + unsigned long section_nr, pfn;
> int ret = 1;
> struct memory_block *mem = to_memory_block(dev);
>
> if (mem->state != MEM_ONLINE)
> goto out;
>
> - for (i = 0; i < sections_per_block; i++) {
> - if (!present_section_nr(mem->start_section_nr + i))
> + for (section_nr = 0; section_nr < sections_per_block; section_nr++) {
> + if (!present_section_nr(mem->start_section_nr + section_nr))
> continue;
> - pfn = section_nr_to_pfn(mem->start_section_nr + i);
> + pfn = section_nr_to_pfn(mem->start_section_nr + section_nr);
> ret &= is_mem_section_removable(pfn, PAGES_PER_SECTION);
> }
>
> @@ -695,12 +695,12 @@ static int add_memory_block(unsigned lon
> {
> int ret, section_count = 0;
> struct memory_block *mem;
> - unsigned long i;
> + unsigned long section_nr;
>
> - for (i = base_section_nr;
> - i < base_section_nr + sections_per_block;
> - i++)
> - if (present_section_nr(i))
> + for (section_nr = base_section_nr;
> + section_nr < base_section_nr + sections_per_block;
> + section_nr++)
> + if (present_section_nr(section_nr))
> section_count++;
>
> if (section_count == 0)
> @@ -823,7 +823,7 @@ static const struct attribute_group *mem
> */
> int __init memory_dev_init(void)
> {
> - unsigned long i;
> + unsigned long section_nr;
> int ret;
> int err;
> unsigned long block_sz;
> @@ -840,9 +840,9 @@ int __init memory_dev_init(void)
> * during boot and have been initialized
> */
> mutex_lock(&mem_sysfs_mutex);
> - for (i = 0; i <= __highest_present_section_nr;
> - i += sections_per_block) {
> - err = add_memory_block(i);
> + for (section_nr = 0; section_nr <= __highest_present_section_nr;
> + section_nr += sections_per_block) {
> + err = add_memory_block(section_nr);
> if (!ret)
> ret = err;
> }
> _
>
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH] powerpc: Enable kernel XZ compression option on PPC_85xx
From: Christian Lamparter @ 2019-06-14 19:56 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-kernel, Pawel Dembicki, Paul Mackerras, linuxppc-dev,
Daniel Axtens
In-Reply-To: <f988951c-3077-ab19-81eb-560418468d14@c-s.fr>
On Friday, June 14, 2019 12:06:48 PM CEST Christophe Leroy wrote:
>
> Le 13/06/2019 à 13:42, Michael Ellerman a écrit :
> > Daniel Axtens <dja@axtens.net> writes:
> >> Pawel Dembicki <paweldembicki@gmail.com> writes:
> >>
> >>> Enable kernel XZ compression option on PPC_85xx. Tested with
> >>> simpleImage on TP-Link TL-WDR4900 (Freescale P1014 processor).
> >>>
> >>> Suggested-by: Christian Lamparter <chunkeey@gmail.com>
> >>> Signed-off-by: Pawel Dembicki <paweldembicki@gmail.com>
> >>> ---
> >>> arch/powerpc/Kconfig | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> >>> index 8c1c636308c8..daf4cb968922 100644
> >>> --- a/arch/powerpc/Kconfig
> >>> +++ b/arch/powerpc/Kconfig
> >>> @@ -196,7 +196,7 @@ config PPC
> >>> select HAVE_IOREMAP_PROT
> >>> select HAVE_IRQ_EXIT_ON_IRQ_STACK
> >>> select HAVE_KERNEL_GZIP
> >>> - select HAVE_KERNEL_XZ if PPC_BOOK3S || 44x
> >>> + select HAVE_KERNEL_XZ if PPC_BOOK3S || 44x || PPC_85xx
> >>
> >> (I'm not super well versed in the compression stuff, so apologies if
> >> this is a dumb question.) If it's this simple, is there any reason we
> >> can't turn it on generally, or convert it to a blacklist of platforms
> >> known not to work?
> >
> > For some platforms enabling XZ requires that your u-boot has XZ support,
> > and I'm not very clear on when that support landed in u-boot and what
> > boards have it. And there are boards out there with old/custom u-boots
> > that effectively can't be updated.
>
> I don't think that it has anything to do with u-boot.
> AFAIK, today's mainline U-boot only supports GZIP (by default) and the
> following optional ones: LZO, LZMA, LZ4.
>
> If we want to set additional compression types for u-boot, it is not
> enough to select HAVE_KERNEL_XXXX, we also have to update uImage
> generation scripts.
>
> See the series I sent some time ago:
> https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=104153
> I'll resent it without bzip2 as today's uboot doesn't support bzip2 anymore.
>
> >
> > But as a server guy I don't really know the details of all that very
> > well. So if someone tells me that we should enable XZ for everything, or
> > as you say just black list some platforms, then that's fine by me.
> >
>
> I guess we first need to understand how this is used.
>
to add to the confusion:
The powerpc arch is sort of special since it has the various targets have
different arch/powerpc/boot/wrapper for everyone unfamiliar (people from
ARM or other targets,) please look at:
https://www.kernel.org/doc/Documentation/powerpc/bootwrapper.txt
and see that this is very different from ARM, MIPS, x86, etc.
I think the cuImage*, dtbImage*, simpleImage, etc... wouldn't
be affected if the kernel is compressed by XZ, as in they should
still boot fine, altough XZ takes a bit longer to unpack of course.
However, for the uImage this could spell a problem, however "HAVE_KERNEL_XZ"
does not automatically entail that the wrapper script from above
compresses the generated uimage with LZMAd/xz. Instead this is controlled
by init/Kconfig and the "Kernel compression mode" setting there.
And currently that defaults to CONFIG_KERNEL_GZIP. So the wrapper script
currently gzipped uImages unless the target config overwrites it to
something else (and the target has the right
HAVE_KERNEL_XZ/BZIP2/LZMA/LZO/LZ4/... as well).
Regards,
Christian
^ permalink raw reply
* Re: [PATCH] powerpc: enable a 30-bit ZONE_DMA for 32-bit pmac
From: Larry Finger @ 2019-06-14 20:26 UTC (permalink / raw)
To: Aaro Koskinen, Mathieu Malaterre
Cc: LKML, Paul Mackerras, linuxppc-dev, Christoph Hellwig
In-Reply-To: <20190614191532.GC27145@darkstar.musicnaut.iki.fi>
On 6/14/19 2:15 PM, Aaro Koskinen wrote:
> Hi,
>
> On Fri, Jun 14, 2019 at 09:24:16AM +0200, Mathieu Malaterre wrote:
>> On Thu, Jun 13, 2019 at 10:27 AM Christoph Hellwig <hch@lst.de> wrote:
>>> With the strict dma mask checking introduced with the switch to
>>> the generic DMA direct code common wifi chips on 32-bit powerbooks
>>> stopped working. Add a 30-bit ZONE_DMA to the 32-bit pmac builds
>>> to allow them to reliably allocate dma coherent memory.
>>>
>>> Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
>>> Reported-by: Aaro Koskinen <aaro.koskinen@iki.fi>
>>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>>> ---
>>> arch/powerpc/include/asm/page.h | 7 +++++++
>>> arch/powerpc/mm/mem.c | 3 ++-
>>> arch/powerpc/platforms/powermac/Kconfig | 1 +
>>> 3 files changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
>>> index b8286a2013b4..0d52f57fca04 100644
>>> --- a/arch/powerpc/include/asm/page.h
>>> +++ b/arch/powerpc/include/asm/page.h
>>> @@ -319,6 +319,13 @@ struct vm_area_struct;
>>> #endif /* __ASSEMBLY__ */
>>> #include <asm/slice.h>
>>>
>>> +/*
>>> + * Allow 30-bit DMA for very limited Broadcom wifi chips on many powerbooks.
>>
>> nit: would it be possible to mention explicit reference to b43-legacy.
>> Using b43 on my macmini g4 never showed those symptoms (using
>> 5.2.0-rc2+)
>
> According to Wikipedia Mac mini G4 is limited to 1 GB RAM, so that's
> why you don't see the issue.
He wouldn't see it with b43. Those cards have 32-bit DMA.
Larry
^ permalink raw reply
* Re: [PATCH v4 19/28] docs: powerpc: convert docs to ReST and rename to *.rst
From: Jonathan Corbet @ 2019-06-14 20:36 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Linux Doc Mailing List, linux-pci, Oliver O'Halloran,
Qiang Zhao, linux-scsi, Jiri Slaby, Linas Vepstas,
Andrew Donnellan, Mauro Carvalho Chehab, Manoj N. Kumar,
Bjorn Helgaas, linux-arm-kernel, Matthew R. Ochs, Uma Krishnan,
Sam Bobroff, Greg Kroah-Hartman, linux-kernel, Li Yang,
Andrew Donnellan, Frederic Barrat, Paul Mackerras, linuxppc-dev
In-Reply-To: <63560c1ee7174952e148a353840a17969fe0be2d.1560361364.git.mchehab+samsung@kernel.org>
On Wed, 12 Jun 2019 14:52:55 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
> Convert docs to ReST and add them to the arch-specific
> book.
>
> The conversion here was trivial, as almost every file there
> was already using an elegant format close to ReST standard.
>
> The changes were mostly to mark literal blocks and add a few
> missing section title identifiers.
>
> One note with regards to "--": on Sphinx, this can't be used
> to identify a list, as it will format it badly. This can be
> used, however, to identify a long hyphen - and "---" is an
> even longer one.
>
> At its new index.rst, let's add a :orphan: while this is not linked to
> the main index.rst file, in order to avoid build warnings.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> # cxl
This one fails to apply because ...
[...]
> diff --git a/Documentation/PCI/pci-error-recovery.rst b/Documentation/PCI/pci-error-recovery.rst
> index 83db42092935..acc21ecca322 100644
> --- a/Documentation/PCI/pci-error-recovery.rst
> +++ b/Documentation/PCI/pci-error-recovery.rst
> @@ -403,7 +403,7 @@ That is, the recovery API only requires that:
> .. note::
>
> Implementation details for the powerpc platform are discussed in
> - the file Documentation/powerpc/eeh-pci-error-recovery.txt
> + the file Documentation/powerpc/eeh-pci-error-recovery.rst
>
> As of this writing, there is a growing list of device drivers with
> patches implementing error recovery. Not all of these patches are in
> @@ -422,3 +422,24 @@ That is, the recovery API only requires that:
> - drivers/net/cxgb3
> - drivers/net/s2io.c
> - drivers/net/qlge
> +
> +>>> As of this writing, there is a growing list of device drivers with
> +>>> patches implementing error recovery. Not all of these patches are in
> +>>> mainline yet. These may be used as "examples":
> +>>>
> +>>> drivers/scsi/ipr
> +>>> drivers/scsi/sym53c8xx_2
> +>>> drivers/scsi/qla2xxx
> +>>> drivers/scsi/lpfc
> +>>> drivers/next/bnx2.c
> +>>> drivers/next/e100.c
> +>>> drivers/net/e1000
> +>>> drivers/net/e1000e
> +>>> drivers/net/ixgb
> +>>> drivers/net/ixgbe
> +>>> drivers/net/cxgb3
> +>>> drivers/net/s2io.c
> +>>> drivers/net/qlge
...of this, which has the look of a set of conflict markers that managed
to get committed...?
jon
^ permalink raw reply
* Re: [RESEND PATCH] Documentation/stackprotector: powerpc supports stack protector
From: Jonathan Corbet @ 2019-06-14 20:44 UTC (permalink / raw)
To: Bhupesh Sharma
Cc: arnd, linux-doc, linux-kernel, paulus, bhupesh.linux,
linuxppc-dev
In-Reply-To: <1560161019-3895-1-git-send-email-bhsharma@redhat.com>
On Mon, 10 Jun 2019 15:33:39 +0530
Bhupesh Sharma <bhsharma@redhat.com> wrote:
> powerpc architecture (both 64-bit and 32-bit) supports stack protector
> mechanism since some time now [see commit 06ec27aea9fc ("powerpc/64:
> add stack protector support")].
>
> Update stackprotector arch support documentation to reflect the same.
>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
> ---
> Resend, this time Cc'ing Jonathan and doc-list.
Applied, thanks.
jon
^ permalink raw reply
* Re: [PATCH v4 19/28] docs: powerpc: convert docs to ReST and rename to *.rst
From: Bjorn Helgaas @ 2019-06-14 21:30 UTC (permalink / raw)
To: Jonathan Corbet
Cc: Linux Doc Mailing List, linux-pci, Oliver O'Halloran,
Mauro Carvalho Chehab, Qiang Zhao, linux-scsi, Jiri Slaby,
Linas Vepstas, Andrew Donnellan, Mauro Carvalho Chehab,
Manoj N. Kumar, linux-arm-kernel, Matthew R. Ochs, Uma Krishnan,
Sam Bobroff, Greg Kroah-Hartman, linux-kernel, Li Yang,
Andrew Donnellan, Frederic Barrat, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190614143635.3aff154d@lwn.net>
On Fri, Jun 14, 2019 at 02:36:35PM -0600, Jonathan Corbet wrote:
> On Wed, 12 Jun 2019 14:52:55 -0300
> Mauro Carvalho Chehab <mchehab+samsung@kernel.org> wrote:
>
> > Convert docs to ReST and add them to the arch-specific
> > book.
> >
> > The conversion here was trivial, as almost every file there
> > was already using an elegant format close to ReST standard.
> >
> > The changes were mostly to mark literal blocks and add a few
> > missing section title identifiers.
> >
> > One note with regards to "--": on Sphinx, this can't be used
> > to identify a list, as it will format it badly. This can be
> > used, however, to identify a long hyphen - and "---" is an
> > even longer one.
> >
> > At its new index.rst, let's add a :orphan: while this is not linked to
> > the main index.rst file, in order to avoid build warnings.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> # cxl
>
> This one fails to apply because ...
>
> [...]
>
> > diff --git a/Documentation/PCI/pci-error-recovery.rst b/Documentation/PCI/pci-error-recovery.rst
> > index 83db42092935..acc21ecca322 100644
> > --- a/Documentation/PCI/pci-error-recovery.rst
> > +++ b/Documentation/PCI/pci-error-recovery.rst
> > @@ -403,7 +403,7 @@ That is, the recovery API only requires that:
> > .. note::
> >
> > Implementation details for the powerpc platform are discussed in
> > - the file Documentation/powerpc/eeh-pci-error-recovery.txt
> > + the file Documentation/powerpc/eeh-pci-error-recovery.rst
> >
> > As of this writing, there is a growing list of device drivers with
> > patches implementing error recovery. Not all of these patches are in
> > @@ -422,3 +422,24 @@ That is, the recovery API only requires that:
> > - drivers/net/cxgb3
> > - drivers/net/s2io.c
> > - drivers/net/qlge
> > +
> > +>>> As of this writing, there is a growing list of device drivers with
> > +>>> patches implementing error recovery. Not all of these patches are in
> > +>>> mainline yet. These may be used as "examples":
> > +>>>
> > +>>> drivers/scsi/ipr
> > +>>> drivers/scsi/sym53c8xx_2
> > +>>> drivers/scsi/qla2xxx
> > +>>> drivers/scsi/lpfc
> > +>>> drivers/next/bnx2.c
> > +>>> drivers/next/e100.c
> > +>>> drivers/net/e1000
> > +>>> drivers/net/e1000e
> > +>>> drivers/net/ixgb
> > +>>> drivers/net/ixgbe
> > +>>> drivers/net/cxgb3
> > +>>> drivers/net/s2io.c
> > +>>> drivers/net/qlge
>
> ...of this, which has the look of a set of conflict markers that managed
> to get committed...?
I don't see these conflict markers in my local branch or in
linux-next (next-20190614).
Let me know if I need to do something.
Bjorn
^ permalink raw reply
* Re: [PATCH v3 1/3] powerpc/powernv: Add OPAL API interface to get secureboot state
From: Nayna @ 2019-06-14 22:22 UTC (permalink / raw)
To: Daniel Axtens
Cc: linux-efi, Ard Biesheuvel, Eric Richter, Nayna Jain, linux-kernel,
Mimi Zohar, Claudio Carvalho, Matthew Garret, linuxppc-dev,
Paul Mackerras, Jeremy Kerr, linux-integrity
In-Reply-To: <87d0jipfr9.fsf@dja-thinkpad.axtens.net>
On 06/12/2019 07:04 PM, Daniel Axtens wrote:
> Hi Nayna,
>
>>>> Since OPAL can support different types of backend which can vary in the
>>>> variable interpretation, a new OPAL API call named OPAL_SECVAR_BACKEND, is
>>>> added to retrieve the supported backend version. This helps the consumer
>>>> to know how to interpret the variable.
>>>>
>>> (Firstly, apologies that I haven't got around to asking about this yet!)
>>>
>>> Are pluggable/versioned backend a good idea?
>>>
>>> There are a few things that worry me about the idea:
>>>
>>> - It adds complexity in crypto (or crypto-adjacent) code, and that
>>> increases the likelihood that we'll accidentally add a bug with bad
>>> consequences.
>> Sorry, I think I am not clear on what exactly you mean here.Can you
>> please elaborate or give specifics ?
> Cryptosystems with greater flexibility can have new kinds of
> vulnerabilities arise from the greater complexity. The first sort of
> thing that comes to mind is a downgrade attack like from TLS. I think
> you're protected from this because the mode cannot be negotiatied at run
> time, but in general it's security sensitive code so I'd like it to be
> as simple as possible.
>
>>> - If we are worried about a long-term-future change to how secure-boot
>>> works, would it be better to just add more get/set calls to opal at
>>> the point at which we actually implement the new system?
>> The intention is to avoid to re-implement the key/value interface for
>> each scheme. Do you mean to deprecate the old APIs and add new APIs with
>> every scheme ?
> Yes, because I expect the scheme would change very, very rarely.
So, the design is not making the assumption that a particular scheme
will change often. It is just allowing the flexibility for addition of
new schemes or enhancements if needed.
>
>>> - Under what circumstances would would we change the kernel-visible
>>> behaviour of skiboot? Are we expecting to change the behaviour,
>>> content or names of the variables in future? Otherwise the only
>>> relevant change I can think of is a change to hardware platforms, and
>>> I'm not sure how a change in hardware would lead to change in
>>> behaviour in the kernel. Wouldn't Skiboot hide h/w differences?
>> Backends are intended to be an agreement for firmware, kernel and
>> userspace on what the format of variables are, what variables should be
>> expected, how they should be signed, etc. Though we don't expect it to
>> happen very often, we want to anticipate possible changes in the
>> firmware which may affect the kernel such as new features, support of
>> new authentication mechanisms, addition of new variables. Corresponding
>> skiboot patches are on -
>> https://lists.ozlabs.org/pipermail/skiboot/2019-June/014641.html
> I still feel like this is holding onto ongoing complexity for very
> little gain, but perhaps this is because I can't picture a specific
> change that would actually require a wholesale change to the scheme.
That is the exact reason for having pluggable backend, because we cannot
determine now if there will be a need of new scheme in future or not.
>
> You mention new features, support for new authentication mechanisms, and
> addition of new variables.
>
> - New features is a bit too generic to answer specifically. In general
> I accept that there exists some new feature that would be
> sufficiently backwards-incompatible as to require a new version. I
> just can't think of one off the top of my head and so I'm not
> convinced it's worth the complexity. Did you have something in mind?
That is the idea to keep the design flexible to be able to handle future
additions with maximum reuse. Example, supporting new algorithms or a
different handling of secure variable updates by different vendors.
>
> - By support for new authentication mechanisms, I assume you mean new
> mechanisms for authenticating variable updates? This is communicated
> in edk2 via the attributes field. Looking at patch 5 from the skiboot
> series:
>
> + * When the attribute EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS is set,
> + * then the Data buffer shall begin with an instance of a complete (and
> + * serialized) EFI_VARIABLE_AUTHENTICATION_2 descriptor.
>
> Could a new authentication scheme be communicated by setting a
> different attribute value? Or are we not carrying attributes in the
> metadata blob?
>
> - For addition of new variables, I'm confused as to why this would
> require a new API - wouldn't it just be exposed in the normal way via
> opal_secvar_get(_next)?
Sorry, probably it wasn't clear. By addition of new variables, we meant
that over time we might have to add new "volatile" variables that "fine
tunes" secure boot state. This might impact the kernel if it needs to
understand new variables to define its policies. However, this will not
result in change of API, it will result in change of the version.
>
> I guess I also somewhat object to calling it a 'backend' if we're using
> it as a version scheme. I think the skiboot storage backends are true
> backends - they provide different implementations of the same
> functionality with the same API, but this seems like you're using it to
> indicate different functionality. It seems like we're using it as if it
> were called OPAL_SECVAR_VERSION.
We are changing how we are exposing the version to the kernel. The
version will be exposed as device-tree entry rather than a OPAL runtime
service. We are not tied to the name "backend", we can switch to calling
it as "scheme" unless there is a better name.
Thanks & Regards,
- Nayna
^ permalink raw reply
* Re: [PATCH v3 3/9] powerpc: Introduce FW_FEATURE_ULTRAVISOR
From: Paul Mackerras @ 2019-06-15 7:36 UTC (permalink / raw)
To: Claudio Carvalho
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, linuxppc-dev, Sukadev Bhattiprolu,
Thiago Bauermann, Anshuman Khandual
In-Reply-To: <20190606173614.32090-4-cclaudio@linux.ibm.com>
On Thu, Jun 06, 2019 at 02:36:08PM -0300, Claudio Carvalho wrote:
> This feature tells if the ultravisor firmware is available to handle
> ucalls.
Everything in this patch that depends on CONFIG_PPC_UV should just
depend on CONFIG_PPC_POWERNV instead. The reason is that every host
kernel needs to be able to do the ultracall to set partition table
entry 0, in case it ends up being run on a machine with an ultravisor.
Otherwise we will have the situation where a host kernel may crash
early in boot just because the machine it's booted on happens to have
an ultravisor running. The crash will be a particularly nasty one
because it will happen before we have probed the machine type and
initialized the console; therefore it will just look like the machine
hangs for no discernable reason.
We also need to think about how to provide a way for petitboot to know
whether the kernel it is booting knows how to do a ucall to set its
partition table entry. One suggestion would be to modify
vmlinux.lds.S to add a new PT_NOTE entry in the program header of the
binary with (say) a 64-bit doubleword which is a bitmap indicating
capabilities of the binary. We would define the first bit as
indicating that the kernel knows how to run under an ultravisor.
When running under an ultravisor, petitboot could then look for the
PT_NOTE and the ultravisor-capable bit in it, and if the PT_NOTE is
not there or the bit is zero, put up a dialog warning the user that
the kernel will probably crash early in boot, and asking for explicit
confirmation that the user wants to proceed.
Paul.
^ permalink raw reply
* Re: [PATCH v3 4/9] KVM: PPC: Ultravisor: Add generic ultravisor call handler
From: Paul Mackerras @ 2019-06-15 7:37 UTC (permalink / raw)
To: Claudio Carvalho
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, linuxppc-dev, Sukadev Bhattiprolu,
Thiago Bauermann, Anshuman Khandual
In-Reply-To: <20190606173614.32090-5-cclaudio@linux.ibm.com>
On Thu, Jun 06, 2019 at 02:36:09PM -0300, Claudio Carvalho wrote:
> From: Ram Pai <linuxram@us.ibm.com>
>
> Add the ucall() function, which can be used to make ultravisor calls
> with varied number of in and out arguments. Ultravisor calls can be made
> from the host or guests.
>
> This copies the implementation of plpar_hcall().
Again, we will want all of this on every powernv-capable kernel, since
they will all need to do UV_WRITE_PATE, even if they have no other
support for the ultravisor.
Paul.
^ permalink raw reply
* Re: [PATCH v3 5/9] KVM: PPC: Ultravisor: Use UV_WRITE_PATE ucall to register a PATE
From: Paul Mackerras @ 2019-06-15 7:38 UTC (permalink / raw)
To: Claudio Carvalho
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, Ryan Grimm, linuxppc-dev, Sukadev Bhattiprolu,
Thiago Bauermann, Anshuman Khandual
In-Reply-To: <20190606173614.32090-6-cclaudio@linux.ibm.com>
On Thu, Jun 06, 2019 at 02:36:10PM -0300, Claudio Carvalho wrote:
> From: Michael Anderson <andmike@linux.ibm.com>
>
> When running under an ultravisor, the ultravisor controls the real
> partition table and has it in secure memory where the hypervisor can't
> access it, and therefore we (the HV) have to do a ucall whenever we want
> to update an entry.
>
> The HV still keeps a copy of its view of the partition table in normal
> memory so that the nest MMU can access it.
>
> Both partition tables will have PATE entries for HV and normal virtual
> machines.
As discussed before, all of this should depend only on
CONFIG_PPC_POWERNV.
Paul.
^ permalink raw reply
* Re: [PATCH v3 7/9] KVM: PPC: Ultravisor: Restrict LDBAR access
From: Paul Mackerras @ 2019-06-15 7:43 UTC (permalink / raw)
To: Claudio Carvalho
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, linuxppc-dev, Sukadev Bhattiprolu,
Thiago Bauermann, Anshuman Khandual
In-Reply-To: <20190606173614.32090-8-cclaudio@linux.ibm.com>
On Thu, Jun 06, 2019 at 02:36:12PM -0300, Claudio Carvalho wrote:
> When the ultravisor firmware is available, it takes control over the
> LDBAR register. In this case, thread-imc updates and save/restore
> operations on the LDBAR register are handled by ultravisor.
>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Just a note on the signed-off-by: it's a bit weird to have Ram's
signoff when he is neither the author nor the sender of the patch.
The author is assumed to be Claudio; if that is not correct then the
patch should have a From: line at the start telling us who the author
is, and ideally that person should have a signoff line before
Claudio's signoff as the sender of the patch.
Paul.
^ permalink raw reply
* Re: [PATCH v3 9/9] KVM: PPC: Ultravisor: Check for MSR_S during hv_reset_msr
From: Paul Mackerras @ 2019-06-15 7:47 UTC (permalink / raw)
To: Claudio Carvalho
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, linuxppc-dev, Sukadev Bhattiprolu,
Thiago Bauermann, Anshuman Khandual
In-Reply-To: <20190606173614.32090-10-cclaudio@linux.ibm.com>
On Thu, Jun 06, 2019 at 02:36:14PM -0300, Claudio Carvalho wrote:
> From: Michael Anderson <andmike@linux.ibm.com>
>
> - Check for MSR_S so that kvmppc_set_msr will include. Prior to this
Will include what? "it" maybe?
> change return to guest would not have the S bit set.
>
> - Patch based on comment from Paul Mackerras <pmac@au1.ibm.com>
>
> Signed-off-by: Michael Anderson <andmike@linux.ibm.com>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Acked-by: Paul Mackerras <paulus@ozlabs.org>
but you should reword the commit message fix that first sentence.
Paul.
^ permalink raw reply
* Re: [PATCH v3 8/9] KVM: PPC: Ultravisor: Enter a secure guest
From: Paul Mackerras @ 2019-06-15 7:45 UTC (permalink / raw)
To: Claudio Carvalho
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, linuxppc-dev, Sukadev Bhattiprolu,
Thiago Bauermann, Anshuman Khandual
In-Reply-To: <20190606173614.32090-9-cclaudio@linux.ibm.com>
On Thu, Jun 06, 2019 at 02:36:13PM -0300, Claudio Carvalho wrote:
> From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
>
> To enter a secure guest, we have to go through the ultravisor, therefore
> we do a ucall when we are entering a secure guest.
>
> This change is needed for any sort of entry to the secure guest from the
> hypervisor, whether it is a return from an hcall, a return from a
> hypervisor interrupt, or the first time that a secure guest vCPU is run.
>
> If we are returning from an hcall, the results are already in the
> appropriate registers (R3:12), except for R6,7, which need to be
> restored before doing the ucall (UV_RETURN).
>
> Have fast_guest_return check the kvm_arch.secure_guest field so that a
> new CPU enters UV when started (in response to a RTAS start-cpu call).
>
> Thanks to input from Paul Mackerras, Ram Pai and Mike Anderson.
>
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> [Pass SRR1 in r11 for UV_RETURN, fix kvmppc_msr_interrupt to preserve
> the MSR_S bit]
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> [Fix UV_RETURN token number and arch.secure_guest check]
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> [Update commit message and ret_to_ultra comment]
> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Acked-by: Paul Mackerras <paulus@ozlabs.org>
Paul.
^ permalink raw reply
* Re: [PATCH v1 1/6] mm: Section numbers use the type "unsigned long"
From: Christophe Leroy @ 2019-06-15 8:06 UTC (permalink / raw)
To: Andrew Morton, David Hildenbrand
Cc: Stephen Rothwell, Michal Hocko, Pavel Tatashin, linux-acpi,
Baoquan He, Rafael J. Wysocki, Greg Kroah-Hartman, linuxppc-dev,
linux-kernel, Wei Yang, linux-mm, Mike Rapoport, Arun KS,
Johannes Weiner, Dan Williams, Mel Gorman, Vlastimil Babka,
Oscar Salvador
In-Reply-To: <20190614120036.00ae392e3f210e7bc9ec6960@linux-foundation.org>
Le 14/06/2019 à 21:00, Andrew Morton a écrit :
> On Fri, 14 Jun 2019 12:01:09 +0200 David Hildenbrand <david@redhat.com> wrote:
>
>> We are using a mixture of "int" and "unsigned long". Let's make this
>> consistent by using "unsigned long" everywhere. We'll do the same with
>> memory block ids next.
>>
>> ...
>>
>> - int i, ret, section_count = 0;
>> + unsigned long i;
>>
>> ...
>>
>> - unsigned int i;
>> + unsigned long i;
>
> Maybe I did too much fortran back in the day, but I think the
> expectation is that a variable called "i" has type "int".
>
> This?
>
>
>
> s/unsigned long i/unsigned long section_nr/
From my point of view you degrade readability by doing that.
section_nr_to_pfn(mem->start_section_nr + section_nr);
Three times the word 'section_nr' in one line, is that worth it ? Gives
me headache.
Codying style says the following, which makes full sense in my opinion:
LOCAL variable names should be short, and to the point. If you have
some random integer loop counter, it should probably be called ``i``.
Calling it ``loop_counter`` is non-productive, if there is no chance of it
being mis-understood.
What about just naming it 'nr' if we want to use something else than 'i' ?
Christophe
>
> --- a/drivers/base/memory.c~mm-section-numbers-use-the-type-unsigned-long-fix
> +++ a/drivers/base/memory.c
> @@ -131,17 +131,17 @@ static ssize_t phys_index_show(struct de
> static ssize_t removable_show(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - unsigned long i, pfn;
> + unsigned long section_nr, pfn;
> int ret = 1;
> struct memory_block *mem = to_memory_block(dev);
>
> if (mem->state != MEM_ONLINE)
> goto out;
>
> - for (i = 0; i < sections_per_block; i++) {
> - if (!present_section_nr(mem->start_section_nr + i))
> + for (section_nr = 0; section_nr < sections_per_block; section_nr++) {
> + if (!present_section_nr(mem->start_section_nr + section_nr))
> continue;
> - pfn = section_nr_to_pfn(mem->start_section_nr + i);
> + pfn = section_nr_to_pfn(mem->start_section_nr + section_nr);
> ret &= is_mem_section_removable(pfn, PAGES_PER_SECTION);
> }
>
> @@ -695,12 +695,12 @@ static int add_memory_block(unsigned lon
> {
> int ret, section_count = 0;
> struct memory_block *mem;
> - unsigned long i;
> + unsigned long section_nr;
>
> - for (i = base_section_nr;
> - i < base_section_nr + sections_per_block;
> - i++)
> - if (present_section_nr(i))
> + for (section_nr = base_section_nr;
> + section_nr < base_section_nr + sections_per_block;
> + section_nr++)
> + if (present_section_nr(section_nr))
> section_count++;
>
> if (section_count == 0)
> @@ -823,7 +823,7 @@ static const struct attribute_group *mem
> */
> int __init memory_dev_init(void)
> {
> - unsigned long i;
> + unsigned long section_nr;
> int ret;
> int err;
> unsigned long block_sz;
> @@ -840,9 +840,9 @@ int __init memory_dev_init(void)
> * during boot and have been initialized
> */
> mutex_lock(&mem_sysfs_mutex);
> - for (i = 0; i <= __highest_present_section_nr;
> - i += sections_per_block) {
> - err = add_memory_block(i);
> + for (section_nr = 0; section_nr <= __highest_present_section_nr;
> + section_nr += sections_per_block) {
> + err = add_memory_block(section_nr);
> if (!ret)
> ret = err;
> }
> _
>
^ permalink raw reply
* Re: [PATCH v3 2/4] crypto: talitos - fix hash on SEC1.
From: Christophe Leroy @ 2019-06-15 8:18 UTC (permalink / raw)
To: Horia Geanta, Herbert Xu, David S. Miller
Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <VI1PR0402MB34855C37F53DC1012DAF670798EE0@VI1PR0402MB3485.eurprd04.prod.outlook.com>
Le 14/06/2019 à 13:32, Horia Geanta a écrit :
> On 6/13/2019 3:48 PM, Christophe Leroy wrote:
>> @@ -336,15 +336,18 @@ static void flush_channel(struct device *dev, int ch, int error, int reset_ch)
>> tail = priv->chan[ch].tail;
>> while (priv->chan[ch].fifo[tail].desc) {
>> __be32 hdr;
>> + struct talitos_edesc *edesc;
>>
>> request = &priv->chan[ch].fifo[tail];
>> + edesc = container_of(request->desc, struct talitos_edesc, desc);
> Not needed for all cases, should be moved to the block that uses it.
Ok.
>
>>
>> /* descriptors with their done bits set don't get the error */
>> rmb();
>> if (!is_sec1)
>> hdr = request->desc->hdr;
>> else if (request->desc->next_desc)
>> - hdr = (request->desc + 1)->hdr1;
>> + hdr = ((struct talitos_desc *)
>> + (edesc->buf + edesc->dma_len))->hdr1;
>> else
>> hdr = request->desc->hdr1;
>>
> [snip]
>> @@ -2058,7 +2065,18 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
>> sg_copy_to_buffer(areq->src, nents,
>> ctx_buf + req_ctx->nbuf, offset);
>> req_ctx->nbuf += offset;
>> - req_ctx->psrc = areq->src;
>> + for (sg = areq->src; sg && offset >= sg->length;
>> + offset -= sg->length, sg = sg_next(sg))
>> + ;
>> + if (offset) {
>> + sg_init_table(req_ctx->bufsl, 2);
>> + sg_set_buf(req_ctx->bufsl, sg_virt(sg) + offset,
>> + sg->length - offset);
>> + sg_chain(req_ctx->bufsl, 2, sg_next(sg));
>> + req_ctx->psrc = req_ctx->bufsl;
> Isn't this what scatterwalk_ffwd() does?
Thanks for pointing this, I wasn't aware of that function. Looking at it
it seems to do the same. Unfortunately, some tests fails with 'wrong
result' when using it instead.
Comparing the results of scatterwalk_ffwd() with what I get with my open
codying, I see the following difference:
scatterwalk_ffwd() uses sg_page(sg) + sg->offset + len
while my open codying results in virt_to_page(sg_virt(sg) + len)
When sg->offset + len is greater than PAGE_SIZE, the resulting SG entry
is different allthough valid in both cases. I think this difference
results in sg_copy_to_buffer() failing. I'm still investigating. Any idea ?
Christophe
>
> Horia
>
^ permalink raw reply
* Issue with sg_copy_to_buffer() ? (was Re: [PATCH v3 2/4] crypto: talitos - fix hash on SEC1.)
From: Christophe Leroy @ 2019-06-15 8:37 UTC (permalink / raw)
To: Horia Geanta, Herbert Xu, David S. Miller
Cc: linuxppc-dev@lists.ozlabs.org, linux-crypto@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <e98f196d-a47c-f2ae-e729-fe2613628da7@c-s.fr>
Le 15/06/2019 à 10:18, Christophe Leroy a écrit :
>>> @@ -2058,7 +2065,18 @@ static int ahash_process_req(struct
>>> ahash_request *areq, unsigned int nbytes)
>>> sg_copy_to_buffer(areq->src, nents,
>>> ctx_buf + req_ctx->nbuf, offset);
>>> req_ctx->nbuf += offset;
>>> - req_ctx->psrc = areq->src;
>>> + for (sg = areq->src; sg && offset >= sg->length;
>>> + offset -= sg->length, sg = sg_next(sg))
>>> + ;
>>> + if (offset) {
>>> + sg_init_table(req_ctx->bufsl, 2);
>>> + sg_set_buf(req_ctx->bufsl, sg_virt(sg) + offset,
>>> + sg->length - offset);
>>> + sg_chain(req_ctx->bufsl, 2, sg_next(sg));
>>> + req_ctx->psrc = req_ctx->bufsl;
>> Isn't this what scatterwalk_ffwd() does?
>
> Thanks for pointing this, I wasn't aware of that function. Looking at it
> it seems to do the same. Unfortunately, some tests fails with 'wrong
> result' when using it instead.
>
> Comparing the results of scatterwalk_ffwd() with what I get with my open
> codying, I see the following difference:
>
> scatterwalk_ffwd() uses sg_page(sg) + sg->offset + len
>
> while my open codying results in virt_to_page(sg_virt(sg) + len)
>
> When sg->offset + len is greater than PAGE_SIZE, the resulting SG entry
> is different allthough valid in both cases. I think this difference
> results in sg_copy_to_buffer() failing. I'm still investigating. Any idea ?
>
After adding some dumps, I confirm the suspicion:
My board uses 16k pages.
SG list when not using scatterwalk_ffwd()
[ 64.120540] sg c6386794 page c7fc1c60 offset 22 len 213
[ 64.120579] sg c6386a48 page c7fc1b80 offset 4 len 2
[ 64.120618] sg c6386a5c page c7fc1b00 offset 3 len 17
[ 64.120658] sg c6386a70 page c7fc1b40 offset 2 len 10
SG list when using scatterwalk_ffwd()
[ 64.120743] sg c6386794 page c7fc1c40 offset 16406 len 213
[ 64.120782] sg c6386a48 page c7fc1b80 offset 4 len 2
[ 64.120821] sg c6386a5c page c7fc1b00 offset 3 len 17
[ 64.120861] sg c6386a70 page c7fc1b40 offset 2 len 10
Content of the SG list:
[ 64.120975] 00000000: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.121021] 00000010: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.121067] 00000020: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.121112] 00000030: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.121157] 00000040: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.121202] 00000050: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.121247] 00000060: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 a8 10
[ 64.121292] 00000070: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.121337] 00000080: e8 40 98 f0 48 a0 f8 50 28 00 58 b0 08 60 b8 10
[ 64.121382] 00000090: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.121427] 000000a0: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.121472] 000000b0: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.121517] 000000c0: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.121557] 000000d0: 68 c0 18 30 c8
[ 64.121598] 00000000: 20 78
[ 64.121646] 00000000: d0 28 80 f8 30 88 e0 38 90 e8 40 98 f0 48 a0 f8
[ 64.121684] 00000010: 50
[ 64.121729] 00000000: a8 00 58 b0 08 60 b8 10 68 c0
Content of the buffer after the copy from the list:
[ 64.121790] 00000000: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.121836] 00000010: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.121881] 00000020: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.121927] 00000030: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.121972] 00000040: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.122017] 00000050: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.122062] 00000060: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 a8 10
[ 64.122107] 00000070: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.122152] 00000080: e8 40 98 f0 48 a0 f8 50 28 00 58 b0 08 60 b8 10
[ 64.122197] 00000090: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.122243] 000000a0: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.122288] 000000b0: 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90
[ 64.122333] 000000c0: e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08 60 b8 10
[ 64.122378] 000000d0: 68 c0 18 30 c8 d8 b0 08 60 b8 10 68 c0 18 70 c8
[ 64.122424] 000000e0: 20 78 d0 28 80 d8 30 88 e0 38 90 e8 40 98 f0 48
[ 64.122462] 000000f0: a0 f8
As you can see, the data following the first block should be
20 78 d0 28 80 f8 30 88 e0 38 90 e8 40 98 f0 48 a0 f8 50 a8 00 58 b0 08
60 b8 10 68 c0
Instead it is
d8 b0 08 60 b8 10 68 c0 18 70 c8 20 78 d0 28 80 d8 30 88 e0 38 90 e8 40
98 f0 48 a0 f8
So I guess there is something wrong with sg_copy_to_buffer()
Christophe
^ permalink raw reply
* crypto/NX: Set receive window credits to max number of CRBs in RxFIFO
From: Haren Myneni @ 2019-06-15 8:39 UTC (permalink / raw)
To: Herbert Xu, mpe; +Cc: linuxppc-dev, linux-crypto, stable
System gets checkstop if RxFIFO overruns with more requests than the
maximum possible number of CRBs in FIFO at the same time. So find max
CRBs from FIFO size and set it to receive window credits.
CC: stable@vger.kernel.org # v4.14+
Signed-off-by:Haren Myneni <haren@us.ibm.com>
diff --git a/drivers/crypto/nx/nx-842-powernv.c b/drivers/crypto/nx/nx-842-powernv.c
index 4acbc47..e78ff5c 100644
--- a/drivers/crypto/nx/nx-842-powernv.c
+++ b/drivers/crypto/nx/nx-842-powernv.c
@@ -27,8 +27,6 @@
#define WORKMEM_ALIGN (CRB_ALIGN)
#define CSB_WAIT_MAX (5000) /* ms */
#define VAS_RETRIES (10)
-/* # of requests allowed per RxFIFO at a time. 0 for unlimited */
-#define MAX_CREDITS_PER_RXFIFO (1024)
struct nx842_workmem {
/* Below fields must be properly aligned */
@@ -812,7 +810,11 @@ static int __init vas_cfg_coproc_info(struct device_node *dn, int chip_id,
rxattr.lnotify_lpid = lpid;
rxattr.lnotify_pid = pid;
rxattr.lnotify_tid = tid;
- rxattr.wcreds_max = MAX_CREDITS_PER_RXFIFO;
+ /*
+ * Maximum RX window credits can not be more than #CRBs in
+ * RxFIFO. Otherwise, can get checkstop if RxFIFO overruns.
+ */
+ rxattr.wcreds_max = fifo_size / CRB_SIZE;
/*
* Open a VAS receice window which is used to configure RxFIFO
^ permalink raw reply related
* Re: [PATCH v5 13/16] powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX
From: Andreas Schwab @ 2019-06-15 11:23 UTC (permalink / raw)
To: Christophe Leroy
Cc: j.neuschaefer, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <1e412310cc18ea654fb2ce4c935654d8d1069f27.1550775950.git.christophe.leroy@c-s.fr>
This breaks suspend (or resume) on the iBook G4. no_console_suspend
doesn't give any clues, the display just stays dark.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH v5 13/16] powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX
From: Andreas Schwab @ 2019-06-15 12:28 UTC (permalink / raw)
To: Christophe Leroy
Cc: j.neuschaefer, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <1e412310cc18ea654fb2ce4c935654d8d1069f27.1550775950.git.christophe.leroy@c-s.fr>
On Feb 21 2019, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index a000768a5cc9..6e56a6240bfa 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -353,7 +353,10 @@ void mark_initmem_nx(void)
> unsigned long numpages = PFN_UP((unsigned long)_einittext) -
> PFN_DOWN((unsigned long)_sinittext);
>
> - change_page_attr(page, numpages, PAGE_KERNEL);
> + if (v_block_mapped((unsigned long)_stext) + 1)
That is always true.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* [PATCH] powerpc/mm/32s: only use MMU to mark initmem NX if STRICT_KERNEL_RWX
From: Andreas Schwab @ 2019-06-15 12:47 UTC (permalink / raw)
To: Christophe Leroy
Cc: j.neuschaefer, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <1e412310cc18ea654fb2ce4c935654d8d1069f27.1550775950.git.christophe.leroy@c-s.fr>
If STRICT_KERNEL_RWX is disabled, never use the MMU to mark initmen
nonexecutable.
Also move a misplaced paren that makes the condition always true.
Fixes: 63b2bc619565 ("powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX")
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
arch/powerpc/mm/pgtable_32.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index d53188dee18f..3935dc263d65 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -360,9 +360,11 @@ void mark_initmem_nx(void)
unsigned long numpages = PFN_UP((unsigned long)_einittext) -
PFN_DOWN((unsigned long)_sinittext);
- if (v_block_mapped((unsigned long)_stext) + 1)
+#ifdef CONFIG_STRICT_KERNEL_RWX
+ if (v_block_mapped((unsigned long)_stext + 1))
mmu_mark_initmem_nx();
else
+#endif
change_page_attr(page, numpages, PAGE_KERNEL);
}
--
2.22.0
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply related
* Re: [PATCH v5 13/16] powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX
From: Christophe Leroy @ 2019-06-15 13:22 UTC (permalink / raw)
To: Andreas Schwab; +Cc: linux-kernel, j.neuschaefer, Paul Mackerras, linuxppc-dev
In-Reply-To: <87blyz9jor.fsf@igel.home>
Andreas Schwab <schwab@linux-m68k.org> a écrit :
> This breaks suspend (or resume) on the iBook G4. no_console_suspend
> doesn't give any clues, the display just stays dark.
Can you send your .config
Thanks
Christophe
>
> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
> "And now for something completely different."
^ permalink raw reply
* Re: [PATCH] powerpc/mm/32s: only use MMU to mark initmem NX if STRICT_KERNEL_RWX
From: Christophe Leroy @ 2019-06-15 13:25 UTC (permalink / raw)
To: Andreas Schwab; +Cc: linux-kernel, j.neuschaefer, Paul Mackerras, linuxppc-dev
In-Reply-To: <8736kb9fry.fsf_-_@igel.home>
Andreas Schwab <schwab@linux-m68k.org> a écrit :
> If STRICT_KERNEL_RWX is disabled, never use the MMU to mark initmen
> nonexecutable.
I dont understand, can you elaborate ?
This area is mapped with BATs so using change_page_attr() is pointless.
Christophe
>
> Also move a misplaced paren that makes the condition always true.
>
> Fixes: 63b2bc619565 ("powerpc/mm/32s: Use BATs for STRICT_KERNEL_RWX")
> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
> ---
> arch/powerpc/mm/pgtable_32.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index d53188dee18f..3935dc263d65 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -360,9 +360,11 @@ void mark_initmem_nx(void)
> unsigned long numpages = PFN_UP((unsigned long)_einittext) -
> PFN_DOWN((unsigned long)_sinittext);
>
> - if (v_block_mapped((unsigned long)_stext) + 1)
> +#ifdef CONFIG_STRICT_KERNEL_RWX
> + if (v_block_mapped((unsigned long)_stext + 1))
> mmu_mark_initmem_nx();
> else
> +#endif
> change_page_attr(page, numpages, PAGE_KERNEL);
> }
>
> --
> 2.22.0
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
> "And now for something completely different."
^ permalink raw reply
* [GIT PULL] Please pull powerpc/linux.git powerpc-5.2-4 tag
From: Michael Ellerman @ 2019-06-15 13:34 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev, linux-kernel, npiggin
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Linus,
Please pull some more powerpc fixes for 5.2:
The following changes since commit cd6c84d8f0cdc911df435bb075ba22ce3c605b07:
Linux 5.2-rc2 (2019-05-26 16:49:19 -0700)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.2-4
for you to fetch changes up to c21f5a9ed85ca3e914ca11f421677ae9ae0d04b0:
powerpc/32s: fix booting with CONFIG_PPC_EARLY_DEBUG_BOOTX (2019-06-07 19:00:14 +1000)
- ------------------------------------------------------------------
powerpc fixes for 5.2 #4
One fix for a regression introduced by our 32-bit KASAN support, which broke
booting on machines with "bootx" early debugging enabled.
A fix for a bug which broke kexec on 32-bit, introduced by changes to the 32-bit
STRICT_KERNEL_RWX support in v5.1.
Finally two fixes going to stable for our THP split/collapse handling,
discovered by Nick. The first fixes random crashes and/or corruption in guests
under sufficient load.
Thanks to:
Nicholas Piggin, Christophe Leroy, Aaro Koskinen, Mathieu Malaterre.
- ------------------------------------------------------------------
Christophe Leroy (2):
powerpc: Fix kexec failure on book3s/32
powerpc/32s: fix booting with CONFIG_PPC_EARLY_DEBUG_BOOTX
Nicholas Piggin (2):
powerpc/64s: Fix THP PMD collapse serialisation
powerpc/64s: __find_linux_pte() synchronization vs pmdp_invalidate()
arch/powerpc/include/asm/book3s/64/pgtable.h | 30 ++++++++++++++++++++++++++++
arch/powerpc/include/asm/btext.h | 4 ++++
arch/powerpc/include/asm/kexec.h | 3 +++
arch/powerpc/kernel/machine_kexec_32.c | 4 +++-
arch/powerpc/kernel/prom_init.c | 1 +
arch/powerpc/kernel/prom_init_check.sh | 2 +-
arch/powerpc/mm/book3s64/pgtable.c | 3 +++
arch/powerpc/mm/pgtable.c | 16 +++++++++++++--
8 files changed, 59 insertions(+), 4 deletions(-)
-----BEGIN PGP SIGNATURE-----
iQIcBAEBAgAGBQJdBO4+AAoJEFHr6jzI4aWAsAwQAK+CK0jvw2pgZMk8/RwuPihJ
gr6pvRaiUuyyiCpWxpzHslZx0WYSg84EYaog4e3fRss6MZeTd4CxxJqAIIny2XTK
3Z6EI3GQGtA8U/+GY+whaQ5+ILdJotbPNRci+yGwc3HNZwT/4RScbmJz7E84MZv+
9gyXrKUio0RdtdZmMHtkrCbpg24QYf1+168gUlJ8H5XGy5NVXVhXwxbYcFeN4zIY
JI+exlBZwtYBJQMtR0FCvjybKk7kRdQzrrUEFM/ZmzsXQryUR7tLrwqAeLvcDc6x
CY9/fn2q7BcFRiOxeZ3AGG89NRTGdOOC1cNJ+Wqn8bIxzP/yFwTEr+lcbdpooCAs
MYyR0yoiI8Aty55lH0uTYQDbXWBZigvKDjLJzn3KN91NKnb3Yw37y5fM5e1ZYQez
bJmbcJJpQzv0YVxXpxd27QeLQtJe6B8D5y0HkpRzYifma5ItAzc1VGzp66jLRFT+
m4LmzD3TjQ61LWyxxDBjAWCHUKW7+cu++sFw0LOA2Wib5DjLjhQAu9qXN1sR5704
FXji4jULMajLMhqqMxjwTEatS46THyz2rqOtJ5/eRWOHBMBS8rHTbHRtFF20mL7x
tHtDmKCfFs2HwHOndtaWduBjiGVVwOo84o2jY0EvfaQ5nscf2XE9acVo6czpJacn
NnIsVZZ6RU/y4Q/f55T4
=Viyv
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH v2] powerpc: pseries/hvconsole: fix stack overread via udbg
From: Michael Ellerman @ 2019-06-15 13:35 UTC (permalink / raw)
To: Daniel Axtens, linuxppc-dev; +Cc: Dmitry Vyukov, Daniel Axtens
In-Reply-To: <20190603065657.7986-1-dja@axtens.net>
On Mon, 2019-06-03 at 06:56:57 UTC, Daniel Axtens wrote:
> While developing kasan for 64-bit book3s, I hit the following stack
> over-read.
>
> It occurs because the hypercall to put characters onto the terminal
> takes 2 longs (128 bits/16 bytes) of characters at a time, and so
> hvc_put_chars would unconditionally copy 16 bytes from the argument
> buffer, regardless of supplied length. However, udbg_hvc_putc can
> call hvc_put_chars with a single-byte buffer, leading to the error.
>
> [ 0.001931] ================================================================== [150/819]
> [ 0.001933] BUG: KASAN: stack-out-of-bounds in hvc_put_chars+0xdc/0x110
> [ 0.001934] Read of size 8 at addr c0000000023e7a90 by task swapper/0
> [ 0.001934]
> [ 0.001935] CPU: 0 PID: 0 Comm: swapper Not tainted 5.2.0-rc2-next-20190528-02824-g048a6ab4835b #113
> [ 0.001935] Call Trace:
> [ 0.001936] [c0000000023e7790] [c000000001b4a450] dump_stack+0x104/0x154 (unreliable)
> [ 0.001937] [c0000000023e77f0] [c0000000006d3524] print_address_description+0xa0/0x30c
> [ 0.001938] [c0000000023e7880] [c0000000006d318c] __kasan_report+0x20c/0x224
> [ 0.001939] [c0000000023e7950] [c0000000006d19d8] kasan_report+0x18/0x30
> [ 0.001940] [c0000000023e7970] [c0000000006d4854] __asan_report_load8_noabort+0x24/0x40
> [ 0.001941] [c0000000023e7990] [c0000000001511ac] hvc_put_chars+0xdc/0x110
> [ 0.001942] [c0000000023e7a10] [c000000000f81cfc] hvterm_raw_put_chars+0x9c/0x110
> [ 0.001943] [c0000000023e7a50] [c000000000f82634] udbg_hvc_putc+0x154/0x200
> [ 0.001944] [c0000000023e7b10] [c000000000049c90] udbg_write+0xf0/0x240
> [ 0.001945] [c0000000023e7b70] [c0000000002e5d88] console_unlock+0x868/0xd30
> [ 0.001946] [c0000000023e7ca0] [c0000000002e6e00] register_console+0x970/0xe90
> [ 0.001947] [c0000000023e7d80] [c000000001ff1928] register_early_udbg_console+0xf8/0x114
> [ 0.001948] [c0000000023e7df0] [c000000001ff1174] setup_arch+0x108/0x790
> [ 0.001948] [c0000000023e7e90] [c000000001fe41c8] start_kernel+0x104/0x784
> [ 0.001949] [c0000000023e7f90] [c00000000000b368] start_here_common+0x1c/0x534
> [ 0.001950]
> [ 0.001950]
> [ 0.001951] Memory state around the buggy address:
> [ 0.001952] c0000000023e7980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 0.001952] c0000000023e7a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1
> [ 0.001953] >c0000000023e7a80: f1 f1 01 f2 f2 f2 00 00 00 00 00 00 00 00 00 00
> [ 0.001953] ^
> [ 0.001954] c0000000023e7b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 0.001954] c0000000023e7b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [ 0.001955] ==================================================================
>
> Document that a 16-byte buffer is requred, and provide it in udbg.
>
> CC: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/934bda59f286d0221f1a3ebab7f5156a
cheers
^ 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