* Re: [PATCH net] ibmvnic: Do not process reset during or after device removal
From: David Miller @ 2019-08-28 22:46 UTC (permalink / raw)
To: tlfalcon; +Cc: netdev, linuxppc-dev
In-Reply-To: <1566922204-8770-1-git-send-email-tlfalcon@linux.ibm.com>
From: Thomas Falcon <tlfalcon@linux.ibm.com>
Date: Tue, 27 Aug 2019 11:10:04 -0500
> Currently, the ibmvnic driver will not schedule device resets
> if the device is being removed, but does not check the device
> state before the reset is actually processed. This leads to a race
> where a reset is scheduled with a valid device state but is
> processed after the driver has been removed, resulting in an oops.
>
> Fix this by checking the device state before processing a queued
> reset event.
>
> Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
> Tested-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
Applied.
^ permalink raw reply
* Re: [PATCH v1] sefltest/ima: support appended signatures (modsig)
From: Thiago Jung Bauermann @ 2019-08-28 23:38 UTC (permalink / raw)
To: Mimi Zohar
Cc: shuah, linuxppc-dev, linux-kernel, Petr Vorel, linux-kselftest,
Jessica Yu, linux-integrity, Dave Young
In-Reply-To: <1567005240-12912-1-git-send-email-zohar@linux.ibm.com>
Hello Mimi,
Mimi Zohar <zohar@linux.ibm.com> writes:
> In addition to the PE/COFF and IMA xattr signatures, the kexec kernel
> image can be signed with an appended signature, using the same
> scripts/sign-file tool that is used to sign kernel modules.
>
> This patch adds support for detecting a kernel image signed with an
> appended signature and updates the existing test messages
> appropriately.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Thanks for doing this!
Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH 01/11] ftrace: move recordmcount tools to scripts/ftrace
From: Changbin Du @ 2019-08-28 23:41 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-arch, linux-s390, linux-parisc, Jonathan Corbet, linux-sh,
John F . Reiser, x86, linux-doc, linux-kernel, linux-mips,
Ingo Molnar, Jessica Yu, sparclinux, linux-kbuild,
Thomas Gleixner, Matt Helsley, linuxppc-dev, linux-riscv,
linux-arm-kernel, Changbin Du
In-Reply-To: <20190826184444.09334ae9@gandalf.local.home>
On Mon, Aug 26, 2019 at 06:44:44PM -0400, Steven Rostedt wrote:
> On Sun, 25 Aug 2019 21:23:20 +0800
> Changbin Du <changbin.du@gmail.com> wrote:
>
> > Move ftrace tools to its own directory. We will add another tool later.
> >
> > Cc: John F. Reiser <jreiser@BitWagon.com>
> > Signed-off-by: Changbin Du <changbin.du@gmail.com>
> > ---
> > scripts/.gitignore | 1 -
> > scripts/Makefile | 2 +-
> > scripts/Makefile.build | 10 +++++-----
> > scripts/ftrace/.gitignore | 4 ++++
> > scripts/ftrace/Makefile | 4 ++++
> > scripts/{ => ftrace}/recordmcount.c | 0
> > scripts/{ => ftrace}/recordmcount.h | 0
> > scripts/{ => ftrace}/recordmcount.pl | 0
> > 8 files changed, 14 insertions(+), 7 deletions(-)
> > create mode 100644 scripts/ftrace/.gitignore
> > create mode 100644 scripts/ftrace/Makefile
> > rename scripts/{ => ftrace}/recordmcount.c (100%)
> > rename scripts/{ => ftrace}/recordmcount.h (100%)
> > rename scripts/{ => ftrace}/recordmcount.pl (100%)
> > mode change 100755 => 100644
>
> Note, we are in the process of merging recordmcount with objtool. It
> would be better to continue from that work.
>
> http://lkml.kernel.org/r/2767f55f4a5fbf30ba0635aed7a9c5ee92ac07dd.1563992889.git.mhelsley@vmware.com
>
> -- Steve
Thanks for reminding. Let me check if prototype tool can merge into
objtool easily after above work.
--
Cheers,
Changbin Du
^ permalink raw reply
* Re: [PATCH v1] sefltest/ima: support appended signatures (modsig)
From: Mimi Zohar @ 2019-08-29 0:01 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: shuah, linuxppc-dev, linux-kernel, Petr Vorel, linux-kselftest,
Jessica Yu, linux-integrity, Dave Young
In-Reply-To: <87o908eu7l.fsf@morokweng.localdomain>
On Wed, 2019-08-28 at 20:38 -0300, Thiago Jung Bauermann wrote:
> Hello Mimi,
>
> Mimi Zohar <zohar@linux.ibm.com> writes:
>
> > In addition to the PE/COFF and IMA xattr signatures, the kexec kernel
> > image can be signed with an appended signature, using the same
> > scripts/sign-file tool that is used to sign kernel modules.
> >
> > This patch adds support for detecting a kernel image signed with an
> > appended signature and updates the existing test messages
> > appropriately.
> >
> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
>
> Thanks for doing this!
You're welcome. This isn't in lieu of a proper regression test that
verifies the IMA measurement list template modsig and d-modsig data
fields. That still needs to be written.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH] PCI: hotplug: Remove surplus return from a void function
From: Tyrel Datwyler @ 2019-08-29 0:11 UTC (permalink / raw)
To: Krzysztof Wilczynski, Scott Murray
Cc: YueHaibing, Sebastian Ott, linux-pci, Rafael J. Wysocki,
linux-kernel, Andy Shevchenko, Lukas Wunner, Bjorn Helgaas,
Paul Mackerras, linuxppc-dev
In-Reply-To: <20190826095143.21353-1-kw@linux.com>
On 8/26/19 2:51 AM, Krzysztof Wilczynski wrote:
> Remove unnecessary empty return statement at the end of a void
> function in the following:
>
> - drivers/pci/hotplug/cpci_hotplug_core.c: cleanup_slots()
> - drivers/pci/hotplug/cpqphp_core.c: pci_print_IRQ_route()
> - drivers/pci/hotplug/cpqphp_ctrl.c: cpqhp_pushbutton_thread()
> - drivers/pci/hotplug/cpqphp_ctrl.c: interrupt_event_handler()
> - drivers/pci/hotplug/cpqphp_nvram.h: compaq_nvram_init()
> - drivers/pci/hotplug/rpadlpar_core.c: rpadlpar_io_init()
> - drivers/pci/hotplug/rpaphp_core.c: cleanup_slots()
>
> Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
For rpa*_core.c portions,
Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com>
> ---
> drivers/pci/hotplug/cpci_hotplug_core.c | 1 -
> drivers/pci/hotplug/cpqphp_core.c | 1 -
> drivers/pci/hotplug/cpqphp_ctrl.c | 4 ----
> drivers/pci/hotplug/cpqphp_nvram.h | 5 +----
> drivers/pci/hotplug/rpadlpar_core.c | 1 -
> drivers/pci/hotplug/rpaphp_core.c | 1 -
> 6 files changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/pci/hotplug/cpci_hotplug_core.c b/drivers/pci/hotplug/cpci_hotplug_core.c
> index 603eadf3d965..d0559d2faf50 100644
> --- a/drivers/pci/hotplug/cpci_hotplug_core.c
> +++ b/drivers/pci/hotplug/cpci_hotplug_core.c
> @@ -563,7 +563,6 @@ cleanup_slots(void)
> }
> cleanup_null:
> up_write(&list_rwsem);
> - return;
> }
>
> int
> diff --git a/drivers/pci/hotplug/cpqphp_core.c b/drivers/pci/hotplug/cpqphp_core.c
> index 16bbb183695a..b8aacb41a83c 100644
> --- a/drivers/pci/hotplug/cpqphp_core.c
> +++ b/drivers/pci/hotplug/cpqphp_core.c
> @@ -173,7 +173,6 @@ static void pci_print_IRQ_route(void)
> dbg("%d %d %d %d\n", tbus, tdevice >> 3, tdevice & 0x7, tslot);
>
> }
> - return;
> }
>
>
> diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c
> index b7f4e1f099d9..68de958a9be8 100644
> --- a/drivers/pci/hotplug/cpqphp_ctrl.c
> +++ b/drivers/pci/hotplug/cpqphp_ctrl.c
> @@ -1872,8 +1872,6 @@ static void interrupt_event_handler(struct controller *ctrl)
> }
> } /* End of FOR loop */
> }
> -
> - return;
> }
>
>
> @@ -1943,8 +1941,6 @@ void cpqhp_pushbutton_thread(struct timer_list *t)
>
> p_slot->state = STATIC_STATE;
> }
> -
> - return;
> }
>
>
> diff --git a/drivers/pci/hotplug/cpqphp_nvram.h b/drivers/pci/hotplug/cpqphp_nvram.h
> index 918ff8dbfe62..70e879b6a23f 100644
> --- a/drivers/pci/hotplug/cpqphp_nvram.h
> +++ b/drivers/pci/hotplug/cpqphp_nvram.h
> @@ -16,10 +16,7 @@
>
> #ifndef CONFIG_HOTPLUG_PCI_COMPAQ_NVRAM
>
> -static inline void compaq_nvram_init(void __iomem *rom_start)
> -{
> - return;
> -}
> +static inline void compaq_nvram_init(void __iomem *rom_start) { }
>
> static inline int compaq_nvram_load(void __iomem *rom_start, struct controller *ctrl)
> {
> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
> index 182f9e3443ee..977946e4e613 100644
> --- a/drivers/pci/hotplug/rpadlpar_core.c
> +++ b/drivers/pci/hotplug/rpadlpar_core.c
> @@ -473,7 +473,6 @@ int __init rpadlpar_io_init(void)
> void rpadlpar_io_exit(void)
> {
> dlpar_sysfs_exit();
> - return;
> }
>
> module_init(rpadlpar_io_init);
> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
> index c3899ee1db99..18627bb21e9e 100644
> --- a/drivers/pci/hotplug/rpaphp_core.c
> +++ b/drivers/pci/hotplug/rpaphp_core.c
> @@ -408,7 +408,6 @@ static void __exit cleanup_slots(void)
> pci_hp_deregister(&slot->hotplug_slot);
> dealloc_slot_struct(slot);
> }
> - return;
> }
>
> static int __init rpaphp_init(void)
>
^ permalink raw reply
* Re: [PATCH v6 00/12] implement KASLR for powerpc/fsl_booke/32
From: Jason Yan @ 2019-08-29 1:57 UTC (permalink / raw)
To: Scott Wood, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
benh, paulus, npiggin, keescook, kernel-hardening
Cc: wangkefeng.wang, linux-kernel, jingxiangfeng, zhaohongjiang,
thunder.leizhen, fanchengyang, yebin10
In-Reply-To: <a39b81562bcdeda7ffe0c2c29a60ff08c77047a6.camel@buserror.net>
On 2019/8/28 12:05, Scott Wood wrote:
> On Fri, 2019-08-09 at 18:07 +0800, Jason Yan wrote:
>> This series implements KASLR for powerpc/fsl_booke/32, as a security
>> feature that deters exploit attempts relying on knowledge of the location
>> of kernel internals.
>>
>> Since CONFIG_RELOCATABLE has already supported, what we need to do is
>> map or copy kernel to a proper place and relocate.
>
> Have you tested this with a kernel that was loaded at a non-zero address? I
> tried loading a kernel at 0x04000000 (by changing the address in the uImage,
> and setting bootm_low to 04000000 in U-Boot), and it works without
> CONFIG_RANDOMIZE and fails with.
>
Not yet. I will test this kind of cases in the next days. Thank you so
much. If there are any other corner cases that have to be tested, please
let me know.
>> Freescale Book-E
>> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
>> entries are not suitable to map the kernel directly in a randomized
>> region, so we chose to copy the kernel to a proper place and restart to
>> relocate.
>>
>> Entropy is derived from the banner and timer base, which will change every
>> build and boot. This not so much safe so additionally the bootloader may
>> pass entropy via the /chosen/kaslr-seed node in device tree.
>
> How complicated would it be to directly access the HW RNG (if present) that
> early in the boot? It'd be nice if a U-Boot update weren't required (and
> particularly concerning that KASLR would appear to work without a U-Boot
> update, but without decent entropy).
>
> -Scott
>
>
>
> .
>
^ permalink raw reply
* RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a
From: Xiaowei Bao @ 2019-08-29 2:03 UTC (permalink / raw)
To: Andrew Murray
Cc: mark.rutland@arm.com, Roy Zang, arnd@arndb.de,
devicetree@vger.kernel.org, gregkh@linuxfoundation.org,
linuxppc-dev@lists.ozlabs.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, kishon@ti.com, M.h. Lian,
robh+dt@kernel.org, gustavo.pimentel@synopsys.com,
jingoohan1@gmail.com, bhelgaas@google.com, Leo Li,
shawnguo@kernel.org, Mingkai Hu,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190828090105.GR14582@e119886-lin.cambridge.arm.com>
> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月28日 17:01
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> lorenzo.pieralisi@arm.co; arnd@arndb.de; gregkh@linuxfoundation.org; M.h.
> Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
> Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
>
> On Wed, Aug 28, 2019 at 04:29:32AM +0000, Xiaowei Bao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andrew Murray <andrew.murray@arm.com>
> > > Sent: 2019年8月27日 21:34
> > > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > Cc: bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> > > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> > > lorenzo.pieralisi@arm.co; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h.
> > > Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
> > > Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> > > gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> > > Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support
> > > for ls1088a and ls2088a
> > >
> > > On Mon, Aug 26, 2019 at 09:49:35AM +0000, Xiaowei Bao wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Andrew Murray <andrew.murray@arm.com>
> > > > > Sent: 2019年8月23日 22:28
> > > > > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > > Cc: bhelgaas@google.com; robh+dt@kernel.org;
> > > > > mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> > > > > <leoyang.li@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.co;
> > > > > arnd@arndb.de; gregkh@linuxfoundation.org;
> > > M.h.
> > > > > Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>;
> > > > > Roy Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
> > > > > gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
> > > > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > > linux-arm-kernel@lists.infradead.org;
> > > > > linuxppc-dev@lists.ozlabs.org
> > > > > Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode
> > > > > support for ls1088a and ls2088a
> > > > >
> > > > > On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > > > > > Add PCIe EP mode support for ls1088a and ls2088a, there are
> > > > > > some difference between LS1 and LS2 platform, so refactor the
> > > > > > code of the EP driver.
> > > > > >
> > > > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > > > ---
> > > > > > v2:
> > > > > > - New mechanism for layerscape EP driver.
> > > > >
> > > > > Was there a v1 of this patch?
> > > > >
> > > > > >
> > > > > > drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> > > > > > ++++++++++++++++++++------
> > > > > > 1 file changed, 58 insertions(+), 18 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > index 7ca5fe8..2a66f07 100644
> > > > > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > @@ -20,27 +20,29 @@
> > > > > >
> > > > > > #define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
> > > > > >
> > > > > > -struct ls_pcie_ep {
> > > > > > - struct dw_pcie *pci;
> > > > > > - struct pci_epc_features *ls_epc;
> > > > > > +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > > > > +
> > > > > > +struct ls_pcie_ep_drvdata {
> > > > > > + u32 func_offset;
> > > > > > + const struct dw_pcie_ep_ops *ops;
> > > > > > + const struct dw_pcie_ops *dw_pcie_ops;
> > > > > > };
> > > > > >
> > > > > > -#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > > > > +struct ls_pcie_ep {
> > > > > > + struct dw_pcie *pci;
> > > > > > + struct pci_epc_features *ls_epc;
> > > > > > + const struct ls_pcie_ep_drvdata *drvdata; };
> > > > > >
> > > > > > static int ls_pcie_establish_link(struct dw_pcie *pci) {
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> > > > > > +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> > > > > > .start_link = ls_pcie_establish_link, };
> > > > > >
> > > > > > -static const struct of_device_id ls_pcie_ep_of_match[] = {
> > > > > > - { .compatible = "fsl,ls-pcie-ep",},
> > > > > > - { },
> > > > > > -};
> > > > > > -
> > > > > > static const struct pci_epc_features*
> > > > > > ls_pcie_ep_get_features(struct dw_pcie_ep *ep) { @@ -82,10
> > > > > > +84,44 @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep,
> u8 func_no,
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > > > > > +static unsigned int ls_pcie_ep_func_conf_select(struct
> > > > > > +dw_pcie_ep
> > > *ep,
> > > > > > + u8 func_no)
> > > > > > +{
> > > > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > > > > > + u8 header_type;
> > > > > > +
> > > > > > + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> > > > > > +
> > > > > > + if (header_type & (1 << 7))
> > > > > > + return pcie->drvdata->func_offset * func_no;
> > > > > > + else
> > > > > > + return 0;
> > > > >
> > > > > It looks like there isn't a PCI define for multi function, the
> > > > > nearest I could find was PCI_HEADER_TYPE_MULTIDEVICE in
> > > > > hotplug/ibmphp.h. A comment above the test might be helpful to
> > > > > explain
> > > the test.
> > > >
> > > > OK, I will add a comment above this code.
> > > >
> > > > >
> > > > > As the ls_pcie_ep_drvdata structures are static, the unset
> > > > > .func_offset will be initialised to 0, so you could just drop the test
> above.
> > > >
> > > > Due to the different PCIe controller have different property, e.g.
> > > > PCIe controller1 support multiple function feature, but PCIe
> > > > controller2 don't support this feature, so I need to check which
> > > > controller support it and return the correct offset value, but
> > > > each board only
> > > have one ls_pcie_ep_drvdata, ^_^.
> > >
> > > Yes but if they don't support the feature then func_offset will be 0.
> > >
> > > >
> > > > >
> > > > > However something to the effect of the following may help spot
> > > > > misconfiguration:
> > > > >
> > > > > WARN_ON(func_no && !pcie->drvdata->func_offset); return
> > > > > pcie->drvdata->func_offset * func_no;
> > > > >
> > > > > The WARN is probably quite useful as if you are attempting to
> > > > > use non-zero functions and func_offset isn't set - then things
> > > > > may appear to work normally but actually will break horribly.
> > > >
> > > > As discussion before, I think the func_offset should not depends
> > > > on the function number, even if other platforms of NXP may be use
> > > > write registers way to access the different function config space.
> > >
> > > I agree that func_offset is an optional parameter. But if you are
> > > attempting to determine the offset of a function and you are given a
> > > non-zero function number - then something has gone wrong if func_offset
> is 0.
> >
> > I have understood you means, maybe I need to set a flag in the
> > driver_data struct, because I may add other platform of NXP, these
> > platform use the write register method to access different function, e.g.
> > write func_num to register, then we can access this func_num config space.
> >
> > I will modify the code like this? Do you have better advice?
> > Case1:
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index 004a7e8..8a0d6df 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -23,6 +23,7 @@
> > #define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> >
> > struct ls_pcie_ep_drvdata {
> > + u8 func_config_flag;
> > u32 func_offset;
> > const struct dw_pcie_ep_ops *ops;
> > const struct dw_pcie_ops *dw_pcie_ops;
> > @@ -97,8 +98,14 @@ static unsigned int
> ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> > * Read the Header Type register of config space to check
> > * whether this PCI device support the multiple function.
> > */
> > - if (header_type & (1 << 7))
> > - return pcie->drvdata->func_offset * func_no;
> > + if (header_type & (1 << 7)) {
> > + if (pcie->drvdata->func_config_flag) {
> > + iowrite32((func_num << n), pci->dbi_base +
> PCI_XXXX_XXX);
> > + } else {
> > + WARN_ON(func_no
> && !pcie->drvdata->func_offset);
> > + return pcie->drvdata->func_offset * func_no;
> > + }
> > + }
> >
> > return 0;
> > }
> >
> > Of course, I don't need to set the flag this time, because I don't use
> > the second method(write register method), so the code like this:
> > case2:
> > +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep
> > +*ep,
> > u8 func_no) {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > u8 header_type;
> >
> > of course, this code is not requied, due to the
> > pcie->drvdata->func_offset is 0, but I think this is more clear
> > if use this code.
> > header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> >
> > /*
> > * Read the Header Type register of config space to check
> > * whether this PCI device support the multiple function.
> > */
> > if (header_type & (1 << 7)) {
> > WARN_ON(func_no && !pcie->drvdata->func_offset);
> > return pcie->drvdata->func_offset * func_no;
> > }
> >
> > return 0;
> > }
> >
> > Or like this:
> > Case3:
> > +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep
> > +*ep,
> > u8 func_no) {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> >
> > WARN_ON(func_no && !pcie->drvdata->func_offset);
> > return pcie->drvdata->func_offset * func_no;
>
> This is better. Given there is only currently one method of calculating an offset
> for layerscape, I'd recommend you add additional methods when the need
> arises.
OK, thanks
>
> Thanks,
>
> Andrew Murray
>
> >
> > }
> > Of course, we can return a -1 by adjuring the (func_no &&
> > !pcie->drvdata->func_offset) Valu in case1
> >
> > Thanks
> > Xiaowei
> >
> > >
> > > Thanks,
> > >
> > > Andrew Murray
> > >
> > > >
> > > > I have added the comments above the code, as follow, do you have
> > > > any
> > > advice?
> > > > +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep
> *ep,
> > > > + u8 func_no)
> {
> > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > > > + u8 header_type;
> > > > +
> > > > + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> > > > +
> > > > + /*
> > > > + * Read the Header Type register of config space to check
> > > > + * whether this PCI device support the multiple function.
> > > > + */
> > > > + if (header_type & (1 << 7))
> > > > + return pcie->drvdata->func_offset * func_no;
> > > > +
> > > > + return 0;
> > > > +}
> > > >
> > > > Thanks a lot for your detail comments.
> > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Andrew Murray
> > > > >
> > > > > > +}
> > > > > > +
> > > > > > +static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
> > > > > > .ep_init = ls_pcie_ep_init,
> > > > > > .raise_irq = ls_pcie_ep_raise_irq,
> > > > > > .get_features = ls_pcie_ep_get_features,
> > > > > > + .func_conf_select = ls_pcie_ep_func_conf_select, };
> > > > > > +
> > > > > > +static const struct ls_pcie_ep_drvdata ls1_ep_drvdata = {
> > > > > > + .ops = &ls_pcie_ep_ops,
> > > > > > + .dw_pcie_ops = &dw_ls_pcie_ep_ops, };
> > > > > > +
> > > > > > +static const struct ls_pcie_ep_drvdata ls2_ep_drvdata = {
> > > > > > + .func_offset = 0x20000,
> > > > > > + .ops = &ls_pcie_ep_ops,
> > > > > > + .dw_pcie_ops = &dw_ls_pcie_ep_ops, };
> > > > > > +
> > > > > > +static const struct of_device_id ls_pcie_ep_of_match[] = {
> > > > > > + { .compatible = "fsl,ls1046a-pcie-ep", .data =
> &ls1_ep_drvdata },
> > > > > > + { .compatible = "fsl,ls1088a-pcie-ep", .data =
> &ls2_ep_drvdata },
> > > > > > + { .compatible = "fsl,ls2088a-pcie-ep", .data =
> &ls2_ep_drvdata },
> > > > > > + { },
> > > > > > };
> > > > > >
> > > > > > static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie, @@
> > > > > > -98,7
> > > > > > +134,7 @@ static int __init ls_add_pcie_ep(struct ls_pcie_ep
> > > > > > +*pcie,
> > > > > > int ret;
> > > > > >
> > > > > > ep = &pci->ep;
> > > > > > - ep->ops = &pcie_ep_ops;
> > > > > > + ep->ops = pcie->drvdata->ops;
> > > > > >
> > > > > > res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM,
> > > > > "addr_space");
> > > > > > if (!res)
> > > > > > @@ -137,14 +173,11 @@ static int __init
> > > > > > ls_pcie_ep_probe(struct
> > > > > platform_device *pdev)
> > > > > > if (!ls_epc)
> > > > > > return -ENOMEM;
> > > > > >
> > > > > > - dbi_base = platform_get_resource_byname(pdev,
> > > IORESOURCE_MEM,
> > > > > "regs");
> > > > > > - pci->dbi_base = devm_pci_remap_cfg_resource(dev,
> dbi_base);
> > > > > > - if (IS_ERR(pci->dbi_base))
> > > > > > - return PTR_ERR(pci->dbi_base);
> > > > > > + pcie->drvdata = of_device_get_match_data(dev);
> > > > > >
> > > > > > - pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> > > > > > pci->dev = dev;
> > > > > > - pci->ops = &ls_pcie_ep_ops;
> > > > > > + pci->ops = pcie->drvdata->dw_pcie_ops;
> > > > > > +
> > > > > > pcie->pci = pci;
> > > > > >
> > > > > > ls_epc->linkup_notifier = false, @@ -152,6 +185,13 @@ static
> > > > > > int __init ls_pcie_ep_probe(struct platform_device *pdev)
> > > > > >
> > > > > > pcie->ls_epc = ls_epc;
> > > > > >
> > > > > > + dbi_base = platform_get_resource_byname(pdev,
> > > IORESOURCE_MEM,
> > > > > "regs");
> > > > > > + pci->dbi_base = devm_pci_remap_cfg_resource(dev,
> dbi_base);
> > > > > > + if (IS_ERR(pci->dbi_base))
> > > > > > + return PTR_ERR(pci->dbi_base);
> > > > > > +
> > > > > > + pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
> > > > > > +
> > > > > > platform_set_drvdata(pdev, pcie);
> > > > > >
> > > > > > ret = ls_add_pcie_ep(pcie, pdev);
> > > > > > --
> > > > > > 2.9.5
> > > > > >
^ permalink raw reply
* Re: [PATCH v6 00/12] implement KASLR for powerpc/fsl_booke/32
From: Jason Yan @ 2019-08-29 2:41 UTC (permalink / raw)
To: Scott Wood, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
benh, paulus, npiggin, keescook, kernel-hardening
Cc: wangkefeng.wang, linux-kernel, jingxiangfeng, zhaohongjiang,
thunder.leizhen, fanchengyang, yebin10
In-Reply-To: <143e5a85bc630d2bb0324114e78bedec8fbeb299.camel@buserror.net>
On 2019/8/28 12:59, Scott Wood wrote:
> On Tue, 2019-08-27 at 23:05 -0500, Scott Wood wrote:
>> On Fri, 2019-08-09 at 18:07 +0800, Jason Yan wrote:
>>> Freescale Book-E
>>> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
>>> entries are not suitable to map the kernel directly in a randomized
>>> region, so we chose to copy the kernel to a proper place and restart to
>>> relocate.
>>>
>>> Entropy is derived from the banner and timer base, which will change every
>>> build and boot. This not so much safe so additionally the bootloader may
>>> pass entropy via the /chosen/kaslr-seed node in device tree.
>>
>> How complicated would it be to directly access the HW RNG (if present) that
>> early in the boot? It'd be nice if a U-Boot update weren't required (and
>> particularly concerning that KASLR would appear to work without a U-Boot
>> update, but without decent entropy).
>
> OK, I see that kaslr-seed is used on some other platforms, though arm64 aborts
> KASLR if it doesn't get a seed. I'm not sure if that's better than a loud
> warning message (or if it was a conscious choice rather than just not having
> an alternative implemented), but silently using poor entropy for something
> like this seems bad.
>
It can still make the attacker's cost higher with not so good entropy.
The same strategy exists in X86 when X86 KASLR uses RDTSC if without
X86_FEATURE_RDRAND supported. I agree that having a warning message
looks better for reminding people in this situation.
> -Scott
>
>
>
> .
>
^ permalink raw reply
* Re: [PATCH v7 1/7] kvmppc: Driver to manage pages of secure guest
From: Sukadev Bhattiprolu @ 2019-08-29 3:02 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, linuxppc-dev, hch
In-Reply-To: <20190822102620.21897-2-bharata@linux.ibm.com>
Some minor comments/questions below. Overall, the patches look
fine to me.
> +#include <linux/pagemap.h>
> +#include <linux/migrate.h>
> +#include <linux/kvm_host.h>
> +#include <asm/ultravisor.h>
> +
> +static struct dev_pagemap kvmppc_devm_pgmap;
> +static unsigned long *kvmppc_devm_pfn_bitmap;
> +static DEFINE_SPINLOCK(kvmppc_devm_pfn_lock);
Is this lock protecting just the pfn_bitmap?
> +
> +struct kvmppc_devm_page_pvt {
> + unsigned long *rmap;
> + unsigned int lpid;
> + unsigned long gpa;
> +};
> +
> +/*
> + * Get a free device PFN from the pool
> + *
> + * Called when a normal page is moved to secure memory (UV_PAGE_IN). Device
> + * PFN will be used to keep track of the secure page on HV side.
> + *
> + * @rmap here is the slot in the rmap array that corresponds to @gpa.
> + * Thus a non-zero rmap entry indicates that the corresponding guest
> + * page has become secure, and is not mapped on the HV side.
> + *
> + * NOTE: In this and subsequent functions, we pass around and access
> + * individual elements of kvm_memory_slot->arch.rmap[] without any
> + * protection. Should we use lock_rmap() here?
> + */
> +static struct page *kvmppc_devm_get_page(unsigned long *rmap, unsigned long gpa,
> + unsigned int lpid)
> +{
> + struct page *dpage = NULL;
> + unsigned long bit, devm_pfn;
> + unsigned long flags;
> + struct kvmppc_devm_page_pvt *pvt;
> + unsigned long pfn_last, pfn_first;
> +
> + if (kvmppc_rmap_is_devm_pfn(*rmap))
> + return NULL;
> +
> + pfn_first = kvmppc_devm_pgmap.res.start >> PAGE_SHIFT;
> + pfn_last = pfn_first +
> + (resource_size(&kvmppc_devm_pgmap.res) >> PAGE_SHIFT);
> + spin_lock_irqsave(&kvmppc_devm_pfn_lock, flags);
Blank lines around spin_lock() would help.
> + bit = find_first_zero_bit(kvmppc_devm_pfn_bitmap, pfn_last - pfn_first);
> + if (bit >= (pfn_last - pfn_first))
> + goto out;
> +
> + bitmap_set(kvmppc_devm_pfn_bitmap, bit, 1);
> + devm_pfn = bit + pfn_first;
Can we drop the &kvmppc_devm_pfn_lock here or after the trylock_page()?
Or does it also protect the ->zone_device_data' assignment below as well?
If so, maybe drop the 'pfn_' from the name of the lock?
Besides, we don't seem to hold this lock when accessing ->zone_device_data
in kvmppc_share_page(). Maybe &kvmppc_devm_pfn_lock just protects the bitmap?
> + dpage = pfn_to_page(devm_pfn);
Does this code and hence CONFIG_PPC_UV depend on a specific model like
CONFIG_SPARSEMEM_VMEMMAP?
> +
> + if (!trylock_page(dpage))
> + goto out_clear;
> +
> + *rmap = devm_pfn | KVMPPC_RMAP_DEVM_PFN;
> + pvt = kzalloc(sizeof(*pvt), GFP_ATOMIC);
> + if (!pvt)
> + goto out_unlock;
> + pvt->rmap = rmap;
> + pvt->gpa = gpa;
> + pvt->lpid = lpid;
> + dpage->zone_device_data = pvt;
->zone_device_data is set after locking the dpage here, but in
kvmppc_share_page() and kvmppc_devm_fault_migrate_alloc_and_copy()
it is accessed without locking the page?
> + spin_unlock_irqrestore(&kvmppc_devm_pfn_lock, flags);
> +
> + get_page(dpage);
> + return dpage;
> +
> +out_unlock:
> + unlock_page(dpage);
> +out_clear:
> + bitmap_clear(kvmppc_devm_pfn_bitmap, devm_pfn - pfn_first, 1);
> +out:
> + spin_unlock_irqrestore(&kvmppc_devm_pfn_lock, flags);
> + return NULL;
> +}
> +
> +/*
> + * Alloc a PFN from private device memory pool and copy page from normal
> + * memory to secure memory.
> + */
> +static int
> +kvmppc_devm_migrate_alloc_and_copy(struct migrate_vma *mig,
> + unsigned long *rmap, unsigned long gpa,
> + unsigned int lpid, unsigned long page_shift)
> +{
> + struct page *spage = migrate_pfn_to_page(*mig->src);
> + unsigned long pfn = *mig->src >> MIGRATE_PFN_SHIFT;
> + struct page *dpage;
> +
> + *mig->dst = 0;
> + if (!spage || !(*mig->src & MIGRATE_PFN_MIGRATE))
> + return 0;
> +
> + dpage = kvmppc_devm_get_page(rmap, gpa, lpid);
> + if (!dpage)
> + return -EINVAL;
> +
> + if (spage)
> + uv_page_in(lpid, pfn << page_shift, gpa, 0, page_shift);
> +
> + *mig->dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
> + return 0;
> +}
> +
> +/*
> + * Move page from normal memory to secure memory.
> + */
> +unsigned long
> +kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
> + unsigned long flags, unsigned long page_shift)
> +{
> + unsigned long addr, end;
> + unsigned long src_pfn, dst_pfn;
These are the host frame numbers correct? Trying to distinguish them
from 'gfn' and 'gpa' used in the function.
> + struct migrate_vma mig;
> + struct vm_area_struct *vma;
> + int srcu_idx;
> + unsigned long gfn = gpa >> page_shift;
> + struct kvm_memory_slot *slot;
> + unsigned long *rmap;
> + int ret;
> +
> + if (page_shift != PAGE_SHIFT)
> + return H_P3;
> +
> + if (flags)
> + return H_P2;
> +
> + ret = H_PARAMETER;
> + down_read(&kvm->mm->mmap_sem);
> + srcu_idx = srcu_read_lock(&kvm->srcu);
> + slot = gfn_to_memslot(kvm, gfn);
Can slot be NULL? could be a bug in UV...
> + rmap = &slot->arch.rmap[gfn - slot->base_gfn];
> + addr = gfn_to_hva(kvm, gpa >> page_shift);
Use 'gfn' as the second parameter?
Nit. for consistency with gpa and gfn, maybe rename 'addr' to
'hva' or to match 'end' maybe to 'start'.
Also, can we check 'kvmppc_rmap_is_devm_pfn(*rmap)' here and bail out
if its already shared? We currently do it further down the call chain
in kvmppc_devm_get_page() after doing more work.
> + if (kvm_is_error_hva(addr))
> + goto out;
> +
> + end = addr + (1UL << page_shift);
> + vma = find_vma_intersection(kvm->mm, addr, end);
> + if (!vma || vma->vm_start > addr || vma->vm_end < end)
> + goto out;
> +
> + memset(&mig, 0, sizeof(mig));
> + mig.vma = vma;
> + mig.start = addr;
> + mig.end = end;
> + mig.src = &src_pfn;
> + mig.dst = &dst_pfn;
> +
> + if (migrate_vma_setup(&mig))
> + goto out;
> +
> + if (kvmppc_devm_migrate_alloc_and_copy(&mig, rmap, gpa,
> + kvm->arch.lpid, page_shift))
> + goto out_finalize;
> +
> + migrate_vma_pages(&mig);
> + ret = H_SUCCESS;
> +out_finalize:
> + migrate_vma_finalize(&mig);
> +out:
> + srcu_read_unlock(&kvm->srcu, srcu_idx);
> + up_read(&kvm->mm->mmap_sem);
> + return ret;
> +}
> +
> +/*
> + * Provision a new page on HV side and copy over the contents
> + * from secure memory.
> + */
> +static int
> +kvmppc_devm_fault_migrate_alloc_and_copy(struct migrate_vma *mig,
> + unsigned long page_shift)
> +{
> + struct page *dpage, *spage;
> + struct kvmppc_devm_page_pvt *pvt;
> + unsigned long pfn;
> + int ret;
> +
> + spage = migrate_pfn_to_page(*mig->src);
> + if (!spage || !(*mig->src & MIGRATE_PFN_MIGRATE))
> + return 0;
> + if (!is_zone_device_page(spage))
> + return 0;
What does it mean if its not a zone_device page at this point? Caller
would then proceed to migrage_vma_pages() if we return 0 right?
> +
> + dpage = alloc_page_vma(GFP_HIGHUSER, mig->vma, mig->start);
> + if (!dpage)
> + return -EINVAL;
> + lock_page(dpage);
> + pvt = spage->zone_device_data;
> +
> + pfn = page_to_pfn(dpage);
> + ret = uv_page_out(pvt->lpid, pfn << page_shift, pvt->gpa, 0,
> + page_shift);
> + if (ret == U_SUCCESS)
> + *mig->dst = migrate_pfn(pfn) | MIGRATE_PFN_LOCKED;
> + else {
> + unlock_page(dpage);
> + __free_page(dpage);
> + }
> + return ret;
> +}
> +
> +/*
> + * Fault handler callback when HV touches any page that has been
> + * moved to secure memory, we ask UV to give back the page by
> + * issuing a UV_PAGE_OUT uvcall.
> + *
> + * This eventually results in dropping of device PFN and the newly
> + * provisioned page/PFN gets populated in QEMU page tables.
> + */
> +static vm_fault_t kvmppc_devm_migrate_to_ram(struct vm_fault *vmf)
> +{
> + unsigned long src_pfn, dst_pfn = 0;
> + struct migrate_vma mig;
> + int ret = 0;
> +
> + memset(&mig, 0, sizeof(mig));
> + mig.vma = vmf->vma;
> + mig.start = vmf->address;
> + mig.end = vmf->address + PAGE_SIZE;
> + mig.src = &src_pfn;
> + mig.dst = &dst_pfn;
> +
> + if (migrate_vma_setup(&mig)) {
> + ret = VM_FAULT_SIGBUS;
> + goto out;
> + }
> +
> + if (kvmppc_devm_fault_migrate_alloc_and_copy(&mig, PAGE_SHIFT)) {
> + ret = VM_FAULT_SIGBUS;
> + goto out_finalize;
> + }
> +
> + migrate_vma_pages(&mig);
> +out_finalize:
> + migrate_vma_finalize(&mig);
> +out:
> + return ret;
> +}
> +
> +/*
> + * Release the device PFN back to the pool
> + *
> + * Gets called when secure page becomes a normal page during UV_PAGE_OUT.
Nit: Should that be H_SVM_PAGE_OUT?
> + */
> +static void kvmppc_devm_page_free(struct page *page)
> +{
> + unsigned long pfn = page_to_pfn(page);
> + unsigned long flags;
> + struct kvmppc_devm_page_pvt *pvt;
> +
> + spin_lock_irqsave(&kvmppc_devm_pfn_lock, flags);
> + pvt = page->zone_device_data;
> + page->zone_device_data = NULL;
If the pfn_lock only protects the bitmap, would be better to move
it here?
> +
> + bitmap_clear(kvmppc_devm_pfn_bitmap,
> + pfn - (kvmppc_devm_pgmap.res.start >> PAGE_SHIFT), 1);
> + *pvt->rmap = 0;
> + spin_unlock_irqrestore(&kvmppc_devm_pfn_lock, flags);
> + kfree(pvt);
> +}
> +
> +static const struct dev_pagemap_ops kvmppc_devm_ops = {
> + .page_free = kvmppc_devm_page_free,
> + .migrate_to_ram = kvmppc_devm_migrate_to_ram,
> +};
> +
> +/*
> + * Move page from secure memory to normal memory.
> + */
> +unsigned long
> +kvmppc_h_svm_page_out(struct kvm *kvm, unsigned long gpa,
> + unsigned long flags, unsigned long page_shift)
> +{
> + struct migrate_vma mig;
> + unsigned long addr, end;
> + struct vm_area_struct *vma;
> + unsigned long src_pfn, dst_pfn = 0;
> + int srcu_idx;
> + int ret;
Nit: Not sure its a coding style requirement, but many functions seem
to "sort" these local variables in descending order of line length for
appearance :-) (eg: migrate_vma* functions).
> +
> + if (page_shift != PAGE_SHIFT)
> + return H_P3;
> +
> + if (flags)
> + return H_P2;
> +
> + ret = H_PARAMETER;
> + down_read(&kvm->mm->mmap_sem);
> + srcu_idx = srcu_read_lock(&kvm->srcu);
> + addr = gfn_to_hva(kvm, gpa >> page_shift);
> + if (kvm_is_error_hva(addr))
> + goto out;
> +
> + end = addr + (1UL << page_shift);
> + vma = find_vma_intersection(kvm->mm, addr, end);
> + if (!vma || vma->vm_start > addr || vma->vm_end < end)
> + goto out;
> +
> + memset(&mig, 0, sizeof(mig));
> + mig.vma = vma;
> + mig.start = addr;
> + mig.end = end;
> + mig.src = &src_pfn;
> + mig.dst = &dst_pfn;
> + if (migrate_vma_setup(&mig))
> + goto out;
> +
> + ret = kvmppc_devm_fault_migrate_alloc_and_copy(&mig, page_shift);
> + if (ret)
> + goto out_finalize;
> +
> + migrate_vma_pages(&mig);
> + ret = H_SUCCESS;
Nit: Blank line here?
> +out_finalize:
> + migrate_vma_finalize(&mig);
> +out:
> + srcu_read_unlock(&kvm->srcu, srcu_idx);
> + up_read(&kvm->mm->mmap_sem);
> + return ret;
> +}
> +
> +static u64 kvmppc_get_secmem_size(void)
> +{
> + struct device_node *np;
> + int i, len;
> + const __be32 *prop;
> + u64 size = 0;
> +
> + np = of_find_compatible_node(NULL, NULL, "ibm,uv-firmware");
> + if (!np)
> + goto out;
> +
> + prop = of_get_property(np, "secure-memory-ranges", &len);
> + if (!prop)
> + goto out_put;
> +
> + for (i = 0; i < len / (sizeof(*prop) * 4); i++)
> + size += of_read_number(prop + (i * 4) + 2, 2);
> +
> +out_put:
> + of_node_put(np);
> +out:
> + return size;
> +}
> +
> +int kvmppc_devm_init(void)
> +{
> + int ret = 0;
> + unsigned long size;
> + struct resource *res;
> + void *addr;
> + unsigned long pfn_last, pfn_first;
> +
> + size = kvmppc_get_secmem_size();
> + if (!size) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + res = request_free_mem_region(&iomem_resource, size, "kvmppc_devm");
> + if (IS_ERR(res)) {
> + ret = PTR_ERR(res);
> + goto out;
> + }
> +
> + kvmppc_devm_pgmap.type = MEMORY_DEVICE_PRIVATE;
> + kvmppc_devm_pgmap.res = *res;
> + kvmppc_devm_pgmap.ops = &kvmppc_devm_ops;
> + addr = memremap_pages(&kvmppc_devm_pgmap, -1);
> + if (IS_ERR(addr)) {
> + ret = PTR_ERR(addr);
> + goto out_free_region;
> + }
> +
> + pfn_first = res->start >> PAGE_SHIFT;
> + pfn_last = pfn_first + (resource_size(res) >> PAGE_SHIFT);
> + kvmppc_devm_pfn_bitmap = kcalloc(BITS_TO_LONGS(pfn_last - pfn_first),
> + sizeof(unsigned long), GFP_KERNEL);
> + if (!kvmppc_devm_pfn_bitmap) {
> + ret = -ENOMEM;
> + goto out_unmap;
> + }
> +
> + pr_info("KVMPPC-DEVM: Secure Memory size 0x%lx\n", size);
> + return ret;
Nit: Blank line here?
> +out_unmap:
> + memunmap_pages(&kvmppc_devm_pgmap);
> +out_free_region:
> + release_mem_region(res->start, size);
> +out:
> + return ret;
> +}
> +
> +void kvmppc_devm_free(void)
> +{
> + memunmap_pages(&kvmppc_devm_pgmap);
> + release_mem_region(kvmppc_devm_pgmap.res.start,
> + resource_size(&kvmppc_devm_pgmap.res));
> + kfree(kvmppc_devm_pfn_bitmap);
> +}
> --
> 2.21.0
^ permalink raw reply
* Re: [PATCH v7 2/7] kvmppc: Shared pages support for secure guests
From: Sukadev Bhattiprolu @ 2019-08-29 3:04 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, linuxppc-dev, hch
In-Reply-To: <20190822102620.21897-3-bharata@linux.ibm.com>
> A secure guest will share some of its pages with hypervisor (Eg. virtio
> bounce buffers etc). Support sharing of pages between hypervisor and
> ultravisor.
>
> Once a secure page is converted to shared page, the device page is
> unmapped from the HV side page tables.
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---
> arch/powerpc/include/asm/hvcall.h | 3 ++
> arch/powerpc/kvm/book3s_hv_devm.c | 70 +++++++++++++++++++++++++++++--
> 2 files changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index 2f6b952deb0f..05b8536f6653 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -337,6 +337,9 @@
> #define H_TLB_INVALIDATE 0xF808
> #define H_COPY_TOFROM_GUEST 0xF80C
>
> +/* Flags for H_SVM_PAGE_IN */
> +#define H_PAGE_IN_SHARED 0x1
> +
> /* Platform-specific hcalls used by the Ultravisor */
> #define H_SVM_PAGE_IN 0xEF00
> #define H_SVM_PAGE_OUT 0xEF04
> diff --git a/arch/powerpc/kvm/book3s_hv_devm.c b/arch/powerpc/kvm/book3s_hv_devm.c
> index 13722f27fa7d..6a3229b78fed 100644
> --- a/arch/powerpc/kvm/book3s_hv_devm.c
> +++ b/arch/powerpc/kvm/book3s_hv_devm.c
> @@ -46,6 +46,7 @@ struct kvmppc_devm_page_pvt {
> unsigned long *rmap;
> unsigned int lpid;
> unsigned long gpa;
> + bool skip_page_out;
> };
>
> /*
> @@ -139,6 +140,54 @@ kvmppc_devm_migrate_alloc_and_copy(struct migrate_vma *mig,
> return 0;
> }
>
> +/*
> + * Shares the page with HV, thus making it a normal page.
> + *
> + * - If the page is already secure, then provision a new page and share
> + * - If the page is a normal page, share the existing page
> + *
> + * In the former case, uses the dev_pagemap_ops migrate_to_ram handler
> + * to unmap the device page from QEMU's page tables.
> + */
> +static unsigned long
> +kvmppc_share_page(struct kvm *kvm, unsigned long gpa, unsigned long page_shift)
> +{
> +
> + int ret = H_PARAMETER;
> + struct page *devm_page;
> + struct kvmppc_devm_page_pvt *pvt;
> + unsigned long pfn;
> + unsigned long *rmap;
> + struct kvm_memory_slot *slot;
> + unsigned long gfn = gpa >> page_shift;
> + int srcu_idx;
> +
> + srcu_idx = srcu_read_lock(&kvm->srcu);
> + slot = gfn_to_memslot(kvm, gfn);
> + if (!slot)
> + goto out;
> +
> + rmap = &slot->arch.rmap[gfn - slot->base_gfn];
> + if (kvmppc_rmap_is_devm_pfn(*rmap)) {
> + devm_page = pfn_to_page(*rmap & ~KVMPPC_RMAP_DEVM_PFN);
> + pvt = (struct kvmppc_devm_page_pvt *)
> + devm_page->zone_device_data;
> + pvt->skip_page_out = true;
> + }
> +
> + pfn = gfn_to_pfn(kvm, gpa >> page_shift);
Use 'gfn'?
> + if (is_error_noslot_pfn(pfn))
> + goto out;
> +
> + ret = uv_page_in(kvm->arch.lpid, pfn << page_shift, gpa, 0, page_shift);
> + if (ret == U_SUCCESS)
> + ret = H_SUCCESS;
> + kvm_release_pfn_clean(pfn);
Nit: Blank line?
> +out:
> + srcu_read_unlock(&kvm->srcu, srcu_idx);
> + return ret;
> +}
> +
> /*
> * Move page from normal memory to secure memory.
> */
> @@ -159,9 +208,12 @@ kvmppc_h_svm_page_in(struct kvm *kvm, unsigned long gpa,
> if (page_shift != PAGE_SHIFT)
> return H_P3;
>
> - if (flags)
> + if (flags & ~H_PAGE_IN_SHARED)
> return H_P2;
>
> + if (flags & H_PAGE_IN_SHARED)
> + return kvmppc_share_page(kvm, gpa, page_shift);
> +
> ret = H_PARAMETER;
> down_read(&kvm->mm->mmap_sem);
> srcu_idx = srcu_read_lock(&kvm->srcu);
> @@ -211,7 +263,7 @@ kvmppc_devm_fault_migrate_alloc_and_copy(struct migrate_vma *mig,
> struct page *dpage, *spage;
> struct kvmppc_devm_page_pvt *pvt;
> unsigned long pfn;
> - int ret;
> + int ret = U_SUCCESS;
>
> spage = migrate_pfn_to_page(*mig->src);
> if (!spage || !(*mig->src & MIGRATE_PFN_MIGRATE))
> @@ -226,8 +278,18 @@ kvmppc_devm_fault_migrate_alloc_and_copy(struct migrate_vma *mig,
> pvt = spage->zone_device_data;
>
> pfn = page_to_pfn(dpage);
> - ret = uv_page_out(pvt->lpid, pfn << page_shift, pvt->gpa, 0,
> - page_shift);
> +
> + /*
> + * This same function is used in two cases:
Nit: s/same//
> + * - When HV touches a secure page, for which we do page-out
Better to qualify page-out with "uv page-out"? its kind of counterintuitive
to do a page-out on a fault!
> + * - When a secure page is converted to shared page, we touch
> + * the page to essentially unmap the device page. In this
> + * case we skip page-out.
> + */
> + if (!pvt->skip_page_out)
> + ret = uv_page_out(pvt->lpid, pfn << page_shift, pvt->gpa, 0,
> + page_shift);
> +
> if (ret == U_SUCCESS)
> *mig->dst = migrate_pfn(pfn) | MIGRATE_PFN_LOCKED;
> else {
> --
> 2.21.0
^ permalink raw reply
* Re: [PATCH v7 5/7] kvmppc: Radix changes for secure guest
From: Sukadev Bhattiprolu @ 2019-08-29 3:05 UTC (permalink / raw)
To: Bharata B Rao
Cc: linuxram, cclaudio, kvm-ppc, linux-mm, jglisse, aneesh.kumar,
paulus, linuxppc-dev, hch
In-Reply-To: <20190822102620.21897-6-bharata@linux.ibm.com>
> - After the guest becomes secure, when we handle a page fault of a page
> belonging to SVM in HV, send that page to UV via UV_PAGE_IN.
> - Whenever a page is unmapped on the HV side, inform UV via UV_PAGE_INVAL.
> - Ensure all those routines that walk the secondary page tables of
> the guest don't do so in case of secure VM. For secure guest, the
> active secondary page tables are in secure memory and the secondary
> page tables in HV are freed when guest becomes secure.
>
> Signed-off-by: Bharata B Rao <bharata@linux.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_host.h | 12 ++++++++++++
> arch/powerpc/include/asm/ultravisor-api.h | 1 +
> arch/powerpc/include/asm/ultravisor.h | 5 +++++
> arch/powerpc/kvm/book3s_64_mmu_radix.c | 22 ++++++++++++++++++++++
> arch/powerpc/kvm/book3s_hv_devm.c | 20 ++++++++++++++++++++
> 5 files changed, 60 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 66e5cc8c9759..29333e8de1c4 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -867,6 +867,8 @@ static inline void kvm_arch_vcpu_block_finish(struct kvm_vcpu *vcpu) {}
> #ifdef CONFIG_PPC_UV
> extern int kvmppc_devm_init(void);
> extern void kvmppc_devm_free(void);
> +extern bool kvmppc_is_guest_secure(struct kvm *kvm);
> +extern int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gpa);
> #else
> static inline int kvmppc_devm_init(void)
> {
> @@ -874,6 +876,16 @@ static inline int kvmppc_devm_init(void)
> }
>
> static inline void kvmppc_devm_free(void) {}
> +
> +static inline bool kvmppc_is_guest_secure(struct kvm *kvm)
> +{
> + return false;
> +}
> +
> +static inline int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gpa)
> +{
> + return -EFAULT;
> +}
> #endif /* CONFIG_PPC_UV */
>
> #endif /* __POWERPC_KVM_HOST_H__ */
> diff --git a/arch/powerpc/include/asm/ultravisor-api.h b/arch/powerpc/include/asm/ultravisor-api.h
> index 46b1ee381695..cf200d4ce703 100644
> --- a/arch/powerpc/include/asm/ultravisor-api.h
> +++ b/arch/powerpc/include/asm/ultravisor-api.h
> @@ -29,5 +29,6 @@
> #define UV_UNREGISTER_MEM_SLOT 0xF124
> #define UV_PAGE_IN 0xF128
> #define UV_PAGE_OUT 0xF12C
> +#define UV_PAGE_INVAL 0xF138
>
> #endif /* _ASM_POWERPC_ULTRAVISOR_API_H */
> diff --git a/arch/powerpc/include/asm/ultravisor.h b/arch/powerpc/include/asm/ultravisor.h
> index 719c0c3930b9..b333241bbe4c 100644
> --- a/arch/powerpc/include/asm/ultravisor.h
> +++ b/arch/powerpc/include/asm/ultravisor.h
> @@ -57,4 +57,9 @@ static inline int uv_unregister_mem_slot(u64 lpid, u64 slotid)
> return ucall_norets(UV_UNREGISTER_MEM_SLOT, lpid, slotid);
> }
>
> +static inline int uv_page_inval(u64 lpid, u64 gpa, u64 page_shift)
> +{
> + return ucall_norets(UV_PAGE_INVAL, lpid, gpa, page_shift);
> +}
> +
> #endif /* _ASM_POWERPC_ULTRAVISOR_H */
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c
> index 2d415c36a61d..93ad34e63045 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_radix.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c
> @@ -19,6 +19,8 @@
> #include <asm/pgtable.h>
> #include <asm/pgalloc.h>
> #include <asm/pte-walk.h>
> +#include <asm/ultravisor.h>
> +#include <asm/kvm_host.h>
>
> /*
> * Supported radix tree geometry.
> @@ -915,6 +917,9 @@ int kvmppc_book3s_radix_page_fault(struct kvm_run *run, struct kvm_vcpu *vcpu,
> if (!(dsisr & DSISR_PRTABLE_FAULT))
> gpa |= ea & 0xfff;
>
> + if (kvmppc_is_guest_secure(kvm))
> + return kvmppc_send_page_to_uv(kvm, gpa & PAGE_MASK);
> +
> /* Get the corresponding memslot */
> memslot = gfn_to_memslot(kvm, gfn);
>
> @@ -972,6 +977,11 @@ int kvm_unmap_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
> unsigned long gpa = gfn << PAGE_SHIFT;
> unsigned int shift;
>
> + if (kvmppc_is_guest_secure(kvm)) {
> + uv_page_inval(kvm->arch.lpid, gpa, PAGE_SIZE);
> + return 0;
> + }
If it is a page we share with UV, won't we need to drop the HV mapping
for the page?
> +
> ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
> if (ptep && pte_present(*ptep))
> kvmppc_unmap_pte(kvm, ptep, gpa, shift, memslot,
> @@ -989,6 +999,9 @@ int kvm_age_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
> int ref = 0;
> unsigned long old, *rmapp;
>
> + if (kvmppc_is_guest_secure(kvm))
> + return ref;
> +
> ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
> if (ptep && pte_present(*ptep) && pte_young(*ptep)) {
> old = kvmppc_radix_update_pte(kvm, ptep, _PAGE_ACCESSED, 0,
> @@ -1013,6 +1026,9 @@ int kvm_test_age_radix(struct kvm *kvm, struct kvm_memory_slot *memslot,
> unsigned int shift;
> int ref = 0;
>
> + if (kvmppc_is_guest_secure(kvm))
> + return ref;
> +
> ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
> if (ptep && pte_present(*ptep) && pte_young(*ptep))
> ref = 1;
> @@ -1030,6 +1046,9 @@ static int kvm_radix_test_clear_dirty(struct kvm *kvm,
> int ret = 0;
> unsigned long old, *rmapp;
>
> + if (kvmppc_is_guest_secure(kvm))
> + return ret;
> +
> ptep = __find_linux_pte(kvm->arch.pgtable, gpa, NULL, &shift);
> if (ptep && pte_present(*ptep) && pte_dirty(*ptep)) {
> ret = 1;
> @@ -1082,6 +1101,9 @@ void kvmppc_radix_flush_memslot(struct kvm *kvm,
> unsigned long gpa;
> unsigned int shift;
>
> + if (kvmppc_is_guest_secure(kvm))
> + return;
> +
> gpa = memslot->base_gfn << PAGE_SHIFT;
> spin_lock(&kvm->mmu_lock);
> for (n = memslot->npages; n; --n) {
> diff --git a/arch/powerpc/kvm/book3s_hv_devm.c b/arch/powerpc/kvm/book3s_hv_devm.c
> index 494495806407..19cfad340a51 100644
> --- a/arch/powerpc/kvm/book3s_hv_devm.c
> +++ b/arch/powerpc/kvm/book3s_hv_devm.c
> @@ -49,6 +49,11 @@ struct kvmppc_devm_page_pvt {
> bool skip_page_out;
> };
>
> +bool kvmppc_is_guest_secure(struct kvm *kvm)
> +{
> + return !!(kvm->arch.secure_guest & KVMPPC_SECURE_INIT_DONE);
> +}
> +
> unsigned long kvmppc_h_svm_init_start(struct kvm *kvm)
> {
> struct kvm_memslots *slots;
> @@ -453,6 +458,21 @@ kvmppc_h_svm_page_out(struct kvm *kvm, unsigned long gpa,
> return ret;
> }
>
> +int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gpa)
> +{
> + unsigned long pfn;
> + int ret;
> +
> + pfn = gfn_to_pfn(kvm, gpa >> PAGE_SHIFT);
> + if (is_error_noslot_pfn(pfn))
> + return -EFAULT;
> +
> + ret = uv_page_in(kvm->arch.lpid, pfn << PAGE_SHIFT, gpa, 0, PAGE_SHIFT);
> + kvm_release_pfn_clean(pfn);
> +
> + return (ret == U_SUCCESS) ? RESUME_GUEST : -EFAULT;
> +}
> +
> static u64 kvmppc_get_secmem_size(void)
> {
> struct device_node *np;
> --
> 2.21.0
^ permalink raw reply
* Re: [PATCH v2 07/10] PCI: layerscape: Modify the MSIX to the doorbell way
From: Kishon Vijay Abraham I @ 2019-08-29 5:13 UTC (permalink / raw)
To: Andrew Murray, Xiaowei Bao, gustavo.pimentel@synopsys.com
Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.co,
arnd@arndb.de, devicetree@vger.kernel.org,
gregkh@linuxfoundation.org, linuxppc-dev@lists.ozlabs.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Leo Li,
M.h. Lian, robh+dt@kernel.org,
linux-arm-kernel@lists.infradead.org, jingoohan1@gmail.com,
bhelgaas@google.com, shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <20190827132504.GL14582@e119886-lin.cambridge.arm.com>
Gustavo,
On 27/08/19 6:55 PM, Andrew Murray wrote:
> On Sat, Aug 24, 2019 at 12:08:40AM +0000, Xiaowei Bao wrote:
>>
>>
>>> -----Original Message-----
>>> From: Andrew Murray <andrew.murray@arm.com>
>>> Sent: 2019年8月23日 21:58
>>> To: Xiaowei Bao <xiaowei.bao@nxp.com>
>>> Cc: bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
>>> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
>>> lorenzo.pieralisi@arm.co; arnd@arndb.de; gregkh@linuxfoundation.org; M.h.
>>> Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; Roy
>>> Zang <roy.zang@nxp.com>; jingoohan1@gmail.com;
>>> gustavo.pimentel@synopsys.com; linux-pci@vger.kernel.org;
>>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
>>> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
>>> Subject: Re: [PATCH v2 07/10] PCI: layerscape: Modify the MSIX to the
>>> doorbell way
>>>
>>> On Thu, Aug 22, 2019 at 07:22:39PM +0800, Xiaowei Bao wrote:
>>>> The layerscape platform use the doorbell way to trigger MSIX interrupt
>>>> in EP mode.
>>>>
>>>
>>> I have no problems with this patch, however...
>>>
>>> Are you able to add to this message a reason for why you are making this
>>> change? Did dw_pcie_ep_raise_msix_irq not work when func_no != 0? Or did
>>> it work yet dw_pcie_ep_raise_msix_irq_doorbell is more efficient?
>>
>> The fact is that, this driver is verified in ls1046a platform of NXP before, and ls1046a don't
>> support MSIX feature, so I set the msix_capable of pci_epc_features struct is false,
>> but in other platform, e.g. ls1088a, it support the MSIX feature, I verified the MSIX
>> feature in ls1088a, it is not OK, so I changed to another way. Thanks.
>
> Right, so the existing pci-layerscape-ep.c driver never supported MSIX yet it
> erroneously had a switch case statement to call dw_pcie_ep_raise_msix_irq which
> would never get used.
>
> Now that we're adding a platform with MSIX support the existing
> dw_pcie_ep_raise_msix_irq doesn't work (for this platform) so we are adding a
> different method.
Gustavo, can you confirm dw_pcie_ep_raise_msix_irq() works for designware as it
didn't work for both me and Xiaowei?
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH rebased] powerpc/fadump: when fadump is supported register the fadump sysfs files.
From: Hari Bathini @ 2019-08-29 5:20 UTC (permalink / raw)
To: Michal Suchánek
Cc: Yangtao Li, Mahesh Salgaonkar, linux-kernel, Paul Mackerras,
Hari Bathini, Thomas Gleixner, linuxppc-dev
In-Reply-To: <20190828190721.555b6337@naga>
On 28/08/19 10:37 PM, Michal Suchánek wrote:
> On Tue, 27 Aug 2019 17:57:31 +0530
> Hari Bathini <hbathini@linux.ibm.com> wrote:
>
[...]
>> Also, get rid of the error message when fadump is
>> not supported as it is already taken care of in fadump_reserve_mem() function.
>
> That should not be called in that case, should it?
fadump_reserve_mem() is called during early boot while this is an initcall that happens
later. Not sure if we can make the initcall conditional on fadump support though..
> Anyway, I find the message right next to the message about reserving
> memory for kdump. So it really looks helpful in the log.
The message you see right after memory reservation for kdump is coming from
fadump_reserve_mem() function. This is the repeat of the same message logged
much later...
- Hari
^ permalink raw reply
* Re: [PATCH v2] powerpc/fadump: when fadump is supported register the fadump sysfs files.
From: Hari Bathini @ 2019-08-29 5:28 UTC (permalink / raw)
To: Michal Suchanek, linuxppc-dev
Cc: Yangtao Li, Mahesh Salgaonkar, linux-kernel, Paul Mackerras,
Hari Bathini, Thomas Gleixner
In-Reply-To: <20190828172742.18378-1-msuchanek@suse.de>
On 28/08/19 10:57 PM, Michal Suchanek wrote:
> Currently it is not possible to distinguish the case when fadump is
> supported by firmware and disabled in kernel and completely unsupported
> using the kernel sysfs interface. User can investigate the devicetree
> but it is more reasonable to provide sysfs files in case we get some
> fadumpv2 in the future.
>
> With this patch sysfs files are available whenever fadump is supported
> by firmware.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
[...]
> - if (!fw_dump.fadump_supported) {
> + if (!fw_dump.fadump_supported && fw_dump.fadump_enabled) {
> printk(KERN_ERR "Firmware-assisted dump is not supported on"
> " this hardware\n");
> - return 0;
> }
The above hunk is redundant with similar message already logged during
early boot in fadump_reserve_mem() function. I am not strongly against
this though. So...
Acked-by: Hari Bathini <hbathini@linux.ibm.com>
^ permalink raw reply
* [PATCH v2 1/4] powerpc/vphn: Check for error from hcall_vphn
From: Srikar Dronamraju @ 2019-08-29 5:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, linuxppc-dev, Srikar Dronamraju, Nicholas Piggin
In-Reply-To: <20190829055023.6171-1-srikar@linux.vnet.ibm.com>
There is no value in unpacking associativity, if
H_HOME_NODE_ASSOCIATIVITY hcall has returned an error.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
---
Changelog (v2->v1):
- Split the patch into 2(Suggested by Nathan).
arch/powerpc/platforms/pseries/vphn.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/vphn.c b/arch/powerpc/platforms/pseries/vphn.c
index 3f07bf6..cca474a 100644
--- a/arch/powerpc/platforms/pseries/vphn.c
+++ b/arch/powerpc/platforms/pseries/vphn.c
@@ -82,7 +82,8 @@ long hcall_vphn(unsigned long cpu, u64 flags, __be32 *associativity)
long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
rc = plpar_hcall9(H_HOME_NODE_ASSOCIATIVITY, retbuf, flags, cpu);
- vphn_unpack_associativity(retbuf, associativity);
+ if (rc == H_SUCCESS)
+ vphn_unpack_associativity(retbuf, associativity);
return rc;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 0/4] Early node associativity
From: Srikar Dronamraju @ 2019-08-29 5:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Srikar Dronamraju, Nicholas Piggin, Abdul Haleem,
Satheesh Rajendran, linuxppc-dev
Abdul reported a warning on a shared lpar.
"WARNING: workqueue cpumask: online intersect > possible intersect".
This is because per node workqueue possible mask is set very early in the
boot process even before the system was querying the home node
associativity. However per node workqueue online cpumask gets updated
dynamically. Hence there is a chance when per node workqueue online cpumask
is a superset of per node workqueue possible mask.
Link for v1: https://patchwork.ozlabs.org/patch/1151658
Changelog: v1->v2
- Handled comments from Nathan Lynch.
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Srikar Dronamraju (4):
powerpc/vphn: Check for error from hcall_vphn
powerpc/numa: Handle extra hcall_vphn error cases
powerpc/numa: Early request for home node associativity
powerpc/numa: Remove late request for home node associativity
arch/powerpc/include/asm/topology.h | 4 --
arch/powerpc/kernel/smp.c | 5 ---
arch/powerpc/mm/numa.c | 77 ++++++++++++++++++++++++-----------
arch/powerpc/platforms/pseries/vphn.c | 3 +-
4 files changed, 56 insertions(+), 33 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH v2 3/4] powerpc/numa: Early request for home node associativity
From: Srikar Dronamraju @ 2019-08-29 5:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Satheesh Rajendran, linuxppc-dev, Srikar Dronamraju,
Nicholas Piggin
In-Reply-To: <20190829055023.6171-1-srikar@linux.vnet.ibm.com>
Currently the kernel detects if its running on a shared lpar platform
and requests home node associativity before the scheduler sched_domains
are setup. However between the time NUMA setup is initialized and the
request for home node associativity, workqueue initializes its per node
cpumask. The per node workqueue possible cpumask may turn invalid
after home node associativity resulting in weird situations like
workqueue possible cpumask being a subset of workqueue online cpumask.
This can be fixed by requesting home node associativity earlier just
before NUMA setup. However at the NUMA setup time, kernel may not be in
a position to detect if its running on a shared lpar platform. So
request for home node associativity and if the request fails, fallback
on the device tree property.
While here, fix a problem where of_node_put could be called even when
of_get_cpu_node was not successful.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
---
Changelog (v2->v1):
- Handled comments from Nathan Lynch
* Dont depend on pacas to be setup for the hwid
arch/powerpc/mm/numa.c | 43 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 8fbe57c..de4a1a1 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -461,14 +461,33 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb)
return nid;
}
+static int vphn_get_nid(unsigned long cpu, bool get_hwid)
+{
+ __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
+ long rc, hwid;
+
+ if (get_hwid)
+ hwid = get_hard_smp_processor_id(cpu);
+ else
+ hwid = cpu_to_phys_id[cpu];
+
+ rc = hcall_vphn(hwid, VPHN_FLAG_VCPU, associativity);
+ if (rc == H_SUCCESS)
+ return associativity_to_nid(associativity);
+
+ return NUMA_NO_NODE;
+}
+
/*
* Figure out to which domain a cpu belongs and stick it there.
+ * cpu_to_phys_id is only valid between smp_setup_cpu_maps() and
+ * smp_setup_pacas(). If called outside this window, set get_hwid to true.
* Return the id of the domain used.
*/
-static int numa_setup_cpu(unsigned long lcpu)
+static int numa_setup_cpu(unsigned long lcpu, bool get_hwid)
{
int nid = NUMA_NO_NODE;
- struct device_node *cpu;
+ struct device_node *cpu = NULL;
/*
* If a valid cpu-to-node mapping is already available, use it
@@ -480,6 +499,20 @@ static int numa_setup_cpu(unsigned long lcpu)
return nid;
}
+ /*
+ * On a shared lpar, device tree will not have node associativity.
+ * At this time lppaca, or its __old_status field may not be
+ * updated. Hence kernel cannot detect if its on a shared lpar. So
+ * request an explicit associativity irrespective of whether the
+ * lpar is shared or dedicated. Use the device tree property as a
+ * fallback.
+ */
+ if (firmware_has_feature(FW_FEATURE_VPHN))
+ nid = vphn_get_nid(lcpu, get_hwid);
+
+ if (nid != NUMA_NO_NODE)
+ goto out_present;
+
cpu = of_get_cpu_node(lcpu, NULL);
if (!cpu) {
@@ -491,13 +524,13 @@ static int numa_setup_cpu(unsigned long lcpu)
}
nid = of_node_to_nid_single(cpu);
+ of_node_put(cpu);
out_present:
if (nid < 0 || !node_possible(nid))
nid = first_online_node;
map_cpu_to_node(lcpu, nid);
- of_node_put(cpu);
out:
return nid;
}
@@ -528,7 +561,7 @@ static int ppc_numa_cpu_prepare(unsigned int cpu)
{
int nid;
- nid = numa_setup_cpu(cpu);
+ nid = numa_setup_cpu(cpu, true);
verify_cpu_node_mapping(cpu, nid);
return 0;
}
@@ -875,7 +908,7 @@ void __init mem_topology_setup(void)
reset_numa_cpu_lookup_table();
for_each_present_cpu(cpu)
- numa_setup_cpu(cpu);
+ numa_setup_cpu(cpu, false);
}
void __init initmem_init(void)
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 2/4] powerpc/numa: Handle extra hcall_vphn error cases
From: Srikar Dronamraju @ 2019-08-29 5:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Satheesh Rajendran, linuxppc-dev, Srikar Dronamraju,
Nicholas Piggin
In-Reply-To: <20190829055023.6171-1-srikar@linux.vnet.ibm.com>
Currently code handles H_FUNCTION, H_SUCCESS, H_HARDWARE return codes.
However hcall_vphn can return other return codes. Now it also handles
H_PARAMETER return code. Also the rest return codes are handled under the
default case.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
---
Changelog (v2->v1):
Handled comments from Nathan:
- Split patch from patch 1.
- Corrected a problem where I missed calling stop_topology_update().
- Using pr_err_ratelimited instead of printk.
arch/powerpc/mm/numa.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 50d68d2..8fbe57c 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1191,23 +1191,30 @@ static long vphn_get_associativity(unsigned long cpu,
VPHN_FLAG_VCPU, associativity);
switch (rc) {
+ case H_SUCCESS:
+ dbg("VPHN hcall succeeded. Reset polling...\n");
+ timed_topology_update(0);
+ goto out;
+
case H_FUNCTION:
- printk_once(KERN_INFO
- "VPHN is not supported. Disabling polling...\n");
- stop_topology_update();
+ pr_err_ratelimited("VPHN unsupported. Disabling polling...\n");
break;
case H_HARDWARE:
- printk(KERN_ERR
- "hcall_vphn() experienced a hardware fault "
+ pr_err_ratelimited("hcall_vphn() experienced a hardware fault "
"preventing VPHN. Disabling polling...\n");
- stop_topology_update();
break;
- case H_SUCCESS:
- dbg("VPHN hcall succeeded. Reset polling...\n");
- timed_topology_update(0);
+ case H_PARAMETER:
+ pr_err_ratelimited("hcall_vphn() was passed an invalid parameter. "
+ "Disabling polling...\n");
+ break;
+ default:
+ pr_err_ratelimited("hcall_vphn() returned %ld. Disabling polling...\n"
+ , rc);
break;
}
+ stop_topology_update();
+out:
return rc;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 4/4] powerpc/numa: Remove late request for home node associativity
From: Srikar Dronamraju @ 2019-08-29 5:50 UTC (permalink / raw)
To: Michael Ellerman
Cc: Nathan Lynch, Satheesh Rajendran, linuxppc-dev, Srikar Dronamraju,
Nicholas Piggin
In-Reply-To: <20190829055023.6171-1-srikar@linux.vnet.ibm.com>
With commit ("powerpc/numa: Early request for home node associativity"),
commit 2ea626306810 ("powerpc/topology: Get topology for shared
processors at boot") which was requesting home node associativity
becomes redundant.
Hence remove the late request for home node associativity.
Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/topology.h | 4 ----
arch/powerpc/kernel/smp.c | 5 -----
arch/powerpc/mm/numa.c | 9 ---------
3 files changed, 18 deletions(-)
diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
index 2f7e1ea..9bd396f 100644
--- a/arch/powerpc/include/asm/topology.h
+++ b/arch/powerpc/include/asm/topology.h
@@ -98,7 +98,6 @@ static inline int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
extern int prrn_is_enabled(void);
extern int find_and_online_cpu_nid(int cpu);
extern int timed_topology_update(int nsecs);
-extern void __init shared_proc_topology_init(void);
#else
static inline int start_topology_update(void)
{
@@ -121,9 +120,6 @@ static inline int timed_topology_update(int nsecs)
return 0;
}
-#ifdef CONFIG_SMP
-static inline void shared_proc_topology_init(void) {}
-#endif
#endif /* CONFIG_NUMA && CONFIG_PPC_SPLPAR */
#include <asm-generic/topology.h>
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index ea6adbf..cdd39a0 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1359,11 +1359,6 @@ void __init smp_cpus_done(unsigned int max_cpus)
if (smp_ops && smp_ops->bringup_done)
smp_ops->bringup_done();
- /*
- * On a shared LPAR, associativity needs to be requested.
- * Hence, get numa topology before dumping cpu topology
- */
- shared_proc_topology_init();
dump_numa_cpu_topology();
#ifdef CONFIG_SCHED_SMT
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index de4a1a1..a20617a 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1608,15 +1608,6 @@ int prrn_is_enabled(void)
return prrn_enabled;
}
-void __init shared_proc_topology_init(void)
-{
- if (lppaca_shared_proc(get_lppaca())) {
- bitmap_fill(cpumask_bits(&cpu_associativity_changes_mask),
- nr_cpumask_bits);
- numa_update_cpu_topology(false);
- }
-}
-
static int topology_read(struct seq_file *file, void *v)
{
if (vphn_enabled || prrn_enabled)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/4] fs: always build llseek.
From: Christoph Hellwig @ 2019-08-29 6:22 UTC (permalink / raw)
To: Michal Suchánek
Cc: David Hildenbrand, Dmitry V. Levin, Max Filippov, Paul Mackerras,
Breno Leitao, Michael Neuling, Christoph Hellwig, Firoz Khan,
Allison Randal, Joel Stanley, Nicholas Piggin, Steven Rostedt,
Thomas Gleixner, Hari Bathini, Greg Kroah-Hartman, linux-kernel,
Eric W. Biederman, Andrew Donnellan, linux-fsdevel, Andrew Morton,
linuxppc-dev, Alexander Viro
In-Reply-To: <20190828181540.21fa33a4@naga>
On Wed, Aug 28, 2019 at 06:15:40PM +0200, Michal Suchánek wrote:
> On Wed, 28 Aug 2019 08:15:52 -0700
> Christoph Hellwig <hch@infradead.org> wrote:
>
> > On Tue, Aug 27, 2019 at 10:21:06PM +0200, Michal Suchanek wrote:
> > > 64bit !COMPAT does not build because the llseek syscall is in the tables.
> >
> > Well, this will bloat thinkgs like 64-bit RISC-V for no good reason.
> > Please introduce a WANT_LSEEK like symbol that ppc64 can select instead.
>
> It also builds when llseek is marked as 32bit only in syscall.tbl
>
> It seems it was handled specially in some way before syscall.tbl was
> added, though (removed in ab66dcc76d6ab8fae9d69d149ae38c42605e7fc5)
Independ of if you need it on a purely 64-bit build on powerpc (which
I'll let the experts figure out) it is not needed on a purely 64-bit
build on other platforms. So please make sure it is still built
conditional, just possibly with an opt-in for powerpc.
^ permalink raw reply
* Re: [PATCH v6 06/12] powerpc/fsl_booke/32: implement KASLR infrastructure
From: Jason Yan @ 2019-08-29 6:26 UTC (permalink / raw)
To: Christophe Leroy, Scott Wood
Cc: wangkefeng.wang, keescook, kernel-hardening, thunder.leizhen,
linux-kernel, npiggin, jingxiangfeng, diana.craciun, paulus,
zhaohongjiang, fanchengyang, linuxppc-dev, yebin10
In-Reply-To: <2db76c55-df5f-5ca8-f0a6-bcee75b8edaa@c-s.fr>
On 2019/8/28 13:47, Christophe Leroy wrote:
>
>
> Le 28/08/2019 à 06:54, Scott Wood a écrit :
>> On Fri, Aug 09, 2019 at 06:07:54PM +0800, Jason Yan wrote:
>>> This patch add support to boot kernel from places other than KERNELBASE.
>>> Since CONFIG_RELOCATABLE has already supported, what we need to do is
>>> map or copy kernel to a proper place and relocate. Freescale Book-E
>>> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
>>> entries are not suitable to map the kernel directly in a randomized
>>> region, so we chose to copy the kernel to a proper place and restart to
>>> relocate.
>>>
>>> The offset of the kernel was not randomized yet(a fixed 64M is set). We
>>> will randomize it in the next patch.
>>>
>>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>>> Cc: Diana Craciun <diana.craciun@nxp.com>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>> Cc: Paul Mackerras <paulus@samba.org>
>>> Cc: Nicholas Piggin <npiggin@gmail.com>
>>> Cc: Kees Cook <keescook@chromium.org>
>>> Tested-by: Diana Craciun <diana.craciun@nxp.com>
>>> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>> ---
>>> arch/powerpc/Kconfig | 11 ++++
>>> arch/powerpc/kernel/Makefile | 1 +
>>> arch/powerpc/kernel/early_32.c | 2 +-
>>> arch/powerpc/kernel/fsl_booke_entry_mapping.S | 17 +++--
>>> arch/powerpc/kernel/head_fsl_booke.S | 13 +++-
>>> arch/powerpc/kernel/kaslr_booke.c | 62 +++++++++++++++++++
>>> arch/powerpc/mm/mmu_decl.h | 7 +++
>>> arch/powerpc/mm/nohash/fsl_booke.c | 7 ++-
>>> 8 files changed, 105 insertions(+), 15 deletions(-)
>>> create mode 100644 arch/powerpc/kernel/kaslr_booke.c
>>>
>
> [...]
>
>>> diff --git a/arch/powerpc/kernel/kaslr_booke.c
>>> b/arch/powerpc/kernel/kaslr_booke.c
>>> new file mode 100644
>>> index 000000000000..f8dc60534ac1
>>> --- /dev/null
>>> +++ b/arch/powerpc/kernel/kaslr_booke.c
>>
>> Shouldn't this go under arch/powerpc/mm/nohash?
>>
>>> +/*
>>> + * To see if we need to relocate the kernel to a random offset
>>> + * void *dt_ptr - address of the device tree
>>> + * phys_addr_t size - size of the first memory block
>>> + */
>>> +notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
>>> +{
>>> + unsigned long tlb_virt;
>>> + phys_addr_t tlb_phys;
>>> + unsigned long offset;
>>> + unsigned long kernel_sz;
>>> +
>>> + kernel_sz = (unsigned long)_end - KERNELBASE;
>>
>> Why KERNELBASE and not kernstart_addr?
>>
>>> +
>>> + offset = kaslr_choose_location(dt_ptr, size, kernel_sz);
>>> +
>>> + if (offset == 0)
>>> + return;
>>> +
>>> + kernstart_virt_addr += offset;
>>> + kernstart_addr += offset;
>>> +
>>> + is_second_reloc = 1;
>>> +
>>> + if (offset >= SZ_64M) {
>>> + tlb_virt = round_down(kernstart_virt_addr, SZ_64M);
>>> + tlb_phys = round_down(kernstart_addr, SZ_64M);
>>
>> If kernstart_addr wasn't 64M-aligned before adding offset, then "offset
>>> = SZ_64M" is not necessarily going to detect when you've crossed a
>> mapping boundary.
>>
>>> +
>>> + /* Create kernel map to relocate in */
>>> + create_tlb_entry(tlb_phys, tlb_virt, 1);
>>> + }
>>> +
>>> + /* Copy the kernel to it's new location and run */
>>> + memcpy((void *)kernstart_virt_addr, (void *)KERNELBASE, kernel_sz);
>>> +
>>> + reloc_kernel_entry(dt_ptr, kernstart_virt_addr);
>>> +}
>>
>> After copying, call flush_icache_range() on the destination.
>
> Function copy_and_flush() does the copy and the flush. I think it should
> be used instead of memcpy() + flush_icache_range()
>
Hi Christophe,
Thanks for the suggestion. But I think copy_and_flush() is not included
in fsl booke code, maybe move this function to misc.S?
> Christophe
>
> .
>
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc: permanently include 8xx registers in reg.h
From: Michael Ellerman @ 2019-08-29 6:30 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, segher,
npiggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e3de2a60198c1b648a5da19ba29938a1e365d1f3.1566931178.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Most 8xx registers have specific names, so just include
> reg_8xx.h all the time in reg.h in order to have them defined
> even when CONFIG_PPC_8xx is not selected. This will avoid
> the need for #ifdefs in C code.
>
> Guard SPRN_ICTRL in an #ifdef CONFIG_PPC_8xx as this register
> has same name but different meaning and different spr number as
> another register in the mpc7450.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> ---
> v2: no change
> ---
> arch/powerpc/include/asm/reg.h | 2 --
> arch/powerpc/include/asm/reg_8xx.h | 2 ++
> 2 files changed, 2 insertions(+), 2 deletions(-)
This breaks the ppc64e build unfortunately, presumably due to it
changing the ordering of header inclusion.
In file included from ../arch/powerpc/include/asm/percpu.h:13,
from ../arch/powerpc/include/asm/mmu.h:137,
from ../arch/powerpc/include/asm/reg_8xx.h:8,
from ../arch/powerpc/include/asm/reg.h:28,
from ../arch/powerpc/include/asm/processor.h:9,
from ../include/linux/processor.h:6,
from ../arch/powerpc/include/asm/delay.h:6,
from ../include/linux/delay.h:26,
from ../lib/nmi_backtrace.c:17:
../arch/powerpc/include/asm/paca.h:147:23: error: field 'tcd' has incomplete type
struct tlb_core_data tcd;
^~~
cheers
^ permalink raw reply
* [PATCH 2/2] powerpc/nvdimm: use H_SCM_QUERY hcall on H_OVERLAP error
From: Aneesh Kumar K.V @ 2019-08-29 6:33 UTC (permalink / raw)
To: mpe; +Cc: Vaibhav Jain, Oliver O'Halloran, linuxppc-dev,
Aneesh Kumar K.V
In-Reply-To: <20190829063347.13966-1-aneesh.kumar@linux.ibm.com>
Right now we force an unbind of SCM memory at drcindex on H_OVERLAP error.
This really slows down operations like kexec where we get the H_OVERLAP
error because we don't go through a full hypervisor re init.
H_OVERLAP error for a H_SCM_BIND_MEM hcall indicates that SCM memory at
drc index is already bound. Since we don't specify a logical memory
address for bind hcall, we can use the H_SCM_QUERY hcall to query
the already bound logical address.
Boot time difference with and without patch is:
[ 5.583617] IOMMU table initialized, virtual merging enabled
[ 5.603041] papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Retrying bind after unbinding
[ 301.514221] papr_scm ibm,persistent-memory:ibm,pmemory@44108001: Retrying bind after unbinding
[ 340.057238] hv-24x7: read 1530 catalog entries, created 537 event attrs (0 failures), 275 descs
after fix
[ 5.101572] IOMMU table initialized, virtual merging enabled
[ 5.116984] papr_scm ibm,persistent-memory:ibm,pmemory@44104001: Querying SCM details
[ 5.117223] papr_scm ibm,persistent-memory:ibm,pmemory@44108001: Querying SCM details
[ 5.120530] hv-24x7: read 1530 catalog entries, created 537 event attrs (0 failures), 275 descs
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 26 ++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 220e595cb579..4b74cfe7b334 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -110,6 +110,27 @@ static void drc_pmem_unbind(struct papr_scm_priv *p)
return;
}
+static int drc_pmem_query(struct papr_scm_priv *p)
+{
+ unsigned long ret[PLPAR_HCALL_BUFSIZE];
+ int64_t rc;
+
+
+ rc = plpar_hcall(H_SCM_QUERY_BLOCK_MEM_BINDING, ret,
+ p->drc_index, 0);
+
+ if (rc) {
+ dev_err(&p->pdev->dev, "Failed to bind SCM");
+ return rc;
+ }
+
+ p->bound_addr = ret[0];
+ dev_dbg(&p->pdev->dev, "bound drc 0x%x to %pR\n", p->drc_index, &p->res);
+
+ return 0;
+}
+
+
static int papr_scm_meta_get(struct papr_scm_priv *p,
struct nd_cmd_get_config_data_hdr *hdr)
{
@@ -431,9 +452,8 @@ static int papr_scm_probe(struct platform_device *pdev)
/* If phyp says drc memory still bound then force unbound and retry */
if (rc == H_OVERLAP) {
- dev_warn(&pdev->dev, "Retrying bind after unbinding\n");
- drc_pmem_unbind(p);
- rc = drc_pmem_bind(p);
+ dev_warn(&pdev->dev, "Querying SCM details\n");
+ rc = drc_pmem_query(p);
}
if (rc != H_SUCCESS) {
--
2.21.0
^ permalink raw reply related
* [PATCH 1/2] powerpc/nvdimm: Use HCALL error as the return value
From: Aneesh Kumar K.V @ 2019-08-29 6:33 UTC (permalink / raw)
To: mpe; +Cc: Vaibhav Jain, Oliver O'Halloran, linuxppc-dev,
Aneesh Kumar K.V
This simplifies the error handling and also enable us to switch to
H_SCM_QUERY hcall in a later patch on H_OVERLAP error.
We also do some kernel print formatting fixup in this patch.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 24 ++++++++++-------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index a5ac371a3f06..220e595cb579 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -66,28 +66,22 @@ static int drc_pmem_bind(struct papr_scm_priv *p)
} while (rc == H_BUSY);
if (rc) {
- /* H_OVERLAP needs a separate error path */
- if (rc == H_OVERLAP)
- return -EBUSY;
-
dev_err(&p->pdev->dev, "bind err: %lld\n", rc);
- return -ENXIO;
+ return rc;
}
p->bound_addr = saved;
-
- dev_dbg(&p->pdev->dev, "bound drc %x to %pR\n", p->drc_index, &p->res);
-
+ dev_dbg(&p->pdev->dev, "bound drc 0x%x to %pR\n", p->drc_index, &p->res);
return 0;
}
-static int drc_pmem_unbind(struct papr_scm_priv *p)
+static void drc_pmem_unbind(struct papr_scm_priv *p)
{
unsigned long ret[PLPAR_HCALL_BUFSIZE];
uint64_t token = 0;
int64_t rc;
- dev_dbg(&p->pdev->dev, "unbind drc %x\n", p->drc_index);
+ dev_dbg(&p->pdev->dev, "unbind drc 0x%x\n", p->drc_index);
/* NB: unbind has the same retry requirements as drc_pmem_bind() */
do {
@@ -110,10 +104,10 @@ static int drc_pmem_unbind(struct papr_scm_priv *p)
if (rc)
dev_err(&p->pdev->dev, "unbind error: %lld\n", rc);
else
- dev_dbg(&p->pdev->dev, "unbind drc %x complete\n",
+ dev_dbg(&p->pdev->dev, "unbind drc 0x%x complete\n",
p->drc_index);
- return rc == H_SUCCESS ? 0 : -ENXIO;
+ return;
}
static int papr_scm_meta_get(struct papr_scm_priv *p,
@@ -436,14 +430,16 @@ static int papr_scm_probe(struct platform_device *pdev)
rc = drc_pmem_bind(p);
/* If phyp says drc memory still bound then force unbound and retry */
- if (rc == -EBUSY) {
+ if (rc == H_OVERLAP) {
dev_warn(&pdev->dev, "Retrying bind after unbinding\n");
drc_pmem_unbind(p);
rc = drc_pmem_bind(p);
}
- if (rc)
+ if (rc != H_SUCCESS) {
+ rc = -ENXIO;
goto err;
+ }
/* setup the resource for the newly bound range */
p->res.start = p->bound_addr;
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v5 1/2] powerpc/xmon: Allow listing and clearing breakpoints in read-only mode
From: Daniel Axtens @ 2019-08-29 6:40 UTC (permalink / raw)
To: Christopher M. Riedl, linuxppc-dev, kernel-hardening; +Cc: ajd
In-Reply-To: <20190828034613.14750-2-cmr@informatik.wtf>
Hi Chris,
> Read-only mode should not prevent listing and clearing any active
> breakpoints.
I tested this and it works for me:
Tested-by: Daniel Axtens <dja@axtens.net>
> + if (xmon_is_ro || !scanhex(&a)) {
It took me a while to figure out what this line does: as I understand
it, the 'b' command can also be used to install a breakpoint (as well as
bi/bd). If we are in ro mode or if the input after 'b' doesn't scan as a
hex string, print the list of breakpoints instead. Anyway, I'm now
happy with it, so:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Regards,
Daniel
> /* print all breakpoints */
> printf(" type address\n");
> if (dabr.enabled) {
> --
> 2.23.0
^ 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