* [PATCH] powerpc/rtas_flash: fix a potential buffer overflow
From: Yi Zhuang @ 2021-07-14 1:16 UTC (permalink / raw)
To: benh, paulus; +Cc: hegdevasant, linuxppc-dev, linux-kernel, zhuangyi1
Since snprintf() returns the possible output size instead of the
actual output size, the available flash_msg length returned by
get_validate_flash_msg may exceed the given buffer limit when
simple_read_from_buffer calls copy_to_user
Signed-off-by: Yi Zhuang <zhuangyi1@huawei.com>
---
arch/powerpc/kernel/rtas_flash.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index a99179d83538..4aa6bad28556 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -473,6 +473,10 @@ static int get_validate_flash_msg(struct rtas_validate_flash_t *args_buf,
(args_buf->update_results == VALIDATE_TMP_UPDATE))
n += snprintf(msg + n, msglen - n, "%s\n",
args_buf->buf);
+ if (n >= msglen) {
+ n = msglen;
+ printk(KERN_ERR "FLASH: msg too long.\n");
+ }
} else {
n = sprintf(msg, "%d\n", args_buf->status);
}
--
2.26.0.106.g9fadedd
^ permalink raw reply related
* Re: [PATCH v4 07/11] powerpc/pseries/iommu: Reorganize iommu_table_setparms*() with new helper
From: Alexey Kardashevskiy @ 2021-07-14 8:32 UTC (permalink / raw)
To: Leonardo Brás, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy, Nicolin Chen,
Niklas Schnelle
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b25dff0f130b9ab721b8b524e55a3cd4c244a8f3.camel@gmail.com>
On 13/07/2021 14:47, Leonardo Brás wrote:
> Hello Alexey,
>
> On Fri, 2021-06-18 at 19:26 -0300, Leonardo Brás wrote:
>>>
>>>> + unsigned long liobn,
>>>> unsigned long win_addr,
>>>> + unsigned long
>>>> window_size,
>>>> unsigned long page_shift,
>>>> + unsigned long base,
>>>> struct
>>>> iommu_table_ops *table_ops)
>>>
>>>
>>> iommu_table_setparms() rather than passing 0 around.
>>>
>>> The same comment about "liobn" - set it in
>>> iommu_table_setparms_lpar().
>>> The reviewer will see what field atters in what situation imho.
>>>
>>
>> The idea here was to keep all tbl parameters setting in
>> _iommu_table_setparms (or iommu_table_setparms_common).
>>
>> I understand the idea that each one of those is optional in the other
>> case, but should we keep whatever value is present in the other
>> variable (not zeroing the other variable), or do someting like:
>>
>> tbl->it_index = 0;
>> tbl->it_base = basep;
>> (in iommu_table_setparms)
>>
>> tbl->it_index = liobn;
>> tbl->it_base = 0;
>> (in iommu_table_setparms_lpar)
>>
>
> This one is supposed to be a question, but I missed the question mark.
> Sorry about that.
Ah ok :)
> I would like to get your opinion in this :)
Besides making the "base" parameter a pointer, I really do not have
strong preference, just make it not hurting eyes of a reader, that's all :)
imho in general, rather than answering 5 weeks later, it is more
productive to address whatever comments were made, add comments (in the
code or commit logs) why you are sticking to your initial approach,
rebase and repost the whole thing. Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH v4 10/11] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Alexey Kardashevskiy @ 2021-07-14 8:38 UTC (permalink / raw)
To: Leonardo Brás, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy, Nicolin Chen,
Niklas Schnelle
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <5b8676140f495dbbe3e28ce261e449b885dbae66.camel@gmail.com>
On 13/07/2021 14:36, Leonardo Brás wrote:
> On Tue, 2021-05-11 at 17:57 +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 01/05/2021 02:31, Leonardo Bras wrote:
>>> [...]
>>> pmem_present = dn != NULL;
>>> @@ -1218,8 +1224,12 @@ static bool enable_ddw(struct pci_dev *dev,
>>> struct device_node *pdn)
>>>
>>> mutex_lock(&direct_window_init_mutex);
>>>
>>> - if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset,
>>> &len))
>>> - goto out_unlock;
>>> + if (find_existing_ddw(pdn, &dev->dev.archdata.dma_offset,
>>> &len)) {
>>> + direct_mapping = (len >= max_ram_len);
>>> +
>>> + mutex_unlock(&direct_window_init_mutex);
>>> + return direct_mapping;
>>
>> Does not this break the existing case when direct_mapping==true by
>> skipping setting dev->dev.bus_dma_limit before returning?
>>
>
> Yes, it does. Good catch!
> I changed it to use a flag instead of win64 for return, and now I can
> use the same success exit path for both the new config and the config
> found in list. (out_unlock)
>
>>
>>
>>> + }
>>>
>>> /*
>>> * If we already went through this for a previous function of
>>> @@ -1298,7 +1308,6 @@ static bool enable_ddw(struct pci_dev *dev,
>>> struct device_node *pdn)
>>> goto out_failed;
>>> }
>>> /* verify the window * number of ptes will map the partition
>>> */
>>> - /* check largest block * page size > max memory hotplug addr
>>> */
>>> /*
>>> * The "ibm,pmemory" can appear anywhere in the address
>>> space.
>>> * Assuming it is still backed by page structs, try
>>> MAX_PHYSMEM_BITS
>>> @@ -1320,6 +1329,17 @@ static bool enable_ddw(struct pci_dev *dev,
>>> struct device_node *pdn)
>>> 1ULL << len,
>>> query.largest_available_block,
>>> 1ULL << page_shift);
>>> +
>>> + len = order_base_2(query.largest_available_block <<
>>> page_shift);
>>> + win_name = DMA64_PROPNAME;
>>
>> [1] ....
>>
>>
>>> + } else {
>>> + direct_mapping = true;
>>> + win_name = DIRECT64_PROPNAME;
>>> + }
>>> +
>>> + /* DDW + IOMMU on single window may fail if there is any
>>> allocation */
>>> + if (default_win_removed && !direct_mapping &&
>>> iommu_table_in_use(tbl)) {
>>> + dev_dbg(&dev->dev, "current IOMMU table in use, can't
>>> be replaced.\n");
>>
>>
>> ... remove !direct_mapping and move to [1]?
>
>
> sure, done!
>
>>
>>
>>> goto out_failed;
>>> }
>>>
>>> @@ -1331,8 +1351,7 @@ static bool enable_ddw(struct pci_dev *dev,
>>> struct device_node *pdn)
>>> create.liobn, dn);
>>>
>>> win_addr = ((u64)create.addr_hi << 32) | create.addr_lo;
>>> - win64 = ddw_property_create(DIRECT64_PROPNAME, create.liobn,
>>> win_addr,
>>> - page_shift, len);
>>> + win64 = ddw_property_create(win_name, create.liobn, win_addr,
>>> page_shift, len);
>>> if (!win64) {
>>> dev_info(&dev->dev,
>>> "couldn't allocate property, property name,
>>> or value\n");
>>> @@ -1350,12 +1369,47 @@ static bool enable_ddw(struct pci_dev *dev,
>>> struct device_node *pdn)
>>> if (!window)
>>> goto out_del_prop;
>>>
>>> - ret = walk_system_ram_range(0, memblock_end_of_DRAM() >>
>>> PAGE_SHIFT,
>>> - win64->value,
>>> tce_setrange_multi_pSeriesLP_walk);
>>> - if (ret) {
>>> - dev_info(&dev->dev, "failed to map direct window for
>>> %pOF: %d\n",
>>> - dn, ret);
>>> - goto out_del_list;
>>> + if (direct_mapping) {
>>> + /* DDW maps the whole partition, so enable direct DMA
>>> mapping */
>>> + ret = walk_system_ram_range(0, memblock_end_of_DRAM()
>>>>> PAGE_SHIFT,
>>> + win64->value,
>>> tce_setrange_multi_pSeriesLP_walk);
>>> + if (ret) {
>>> + dev_info(&dev->dev, "failed to map direct
>>> window for %pOF: %d\n",
>>> + dn, ret);
>>> + goto out_del_list;
>>> + }
>>> + } else {
>>> + struct iommu_table *newtbl;
>>> + int i;
>>> +
>>> + /* New table for using DDW instead of the default DMA
>>> window */
>>> + newtbl = iommu_pseries_alloc_table(pci->phb->node);
>>> + if (!newtbl) {
>>> + dev_dbg(&dev->dev, "couldn't create new IOMMU
>>> table\n");
>>> + goto out_del_list;
>>> + }
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(pci->phb->mem_resources);
>>> i++) {
>>> + const unsigned long mask = IORESOURCE_MEM_64
>>> | IORESOURCE_MEM;
>>> +
>>> + /* Look for MMIO32 */
>>> + if ((pci->phb->mem_resources[i].flags & mask)
>>> == IORESOURCE_MEM)
>>> + break;
>>
>> What if there is no IORESOURCE_MEM? pci->phb->mem_resources[i].start
>> below will have garbage.
>
>
>
> Yeah, that makes sense. I will add this lines after 'for':
>
> if (i == ARRAY_SIZE(pci->phb->mem_resources)) {
> iommu_tce_table_put(newtbl);
> goto out_del_list;
> }
>
> What do you think?
Move this and that "for" before iommu_pseries_alloc_table() so you won't
need to free if there is no IORESOURCE_MEM.
>
>
>>
>>
>>> + }
>>> +
>>> + _iommu_table_setparms(newtbl, pci->phb->bus->number,
>>> create.liobn, win_addr,
>>> + 1UL << len, page_shift, 0,
>>> &iommu_table_lpar_multi_ops);
>>> + iommu_init_table(newtbl, pci->phb->node, pci->phb-
>>>> mem_resources[i].start,
>>> + pci->phb->mem_resources[i].end);
>>> +
>>> + if (default_win_removed)
>>> + iommu_tce_table_put(tbl);
>>
>>
>> iommu_tce_table_put() should have been called when the window was
>> removed.
>>
>> Also after some thinking - what happens if there were 2 devices in the
>> PE and one requested 64bit DMA? This will only update
>> set_iommu_table_base() for the 64bit one but not for the other.
>>
>> I think the right thing to do is:
>>
>> 1. check if table[0] is in use, if yes => fail (which this does
>> already)
>>
>> 2. remove default dma window but keep the iommu_table struct with one
>> change - set it_size to 0 (and free it_map) so the 32bit device won't
>> look at a stale structure and think there is some window (imaginery
>> situation for phyp but easy to recreate in qemu).
>>
>> 3. use table[1] for newly created indirect DDW window.
>>
>> 4. change get_iommu_table_base() to return a usable table (or may be
>> not
>> needed?).
>>
>> If this sounds reasonable (does it?),
>
> Looks ok, I will try your suggestion.
> I was not aware of how pci->table_group->tables[] worked, so I replaced
> pci->table_group->tables[0] with the new tbl, while moving the older in
> pci->table_group->tables[1].
pci->table_group->tables[0] is window#0 at @0.
pci->table_group->tables[1] is window#1 at 0x0800.0000.0000.0000. That
is all :)
pseries does not use tables[1] but powernv does (by VFIO only though).
> (4) get_iommu_table_base() does not seem to need update, as it returns
> the tlb set by set_iommu_table_base() which is already called in the
> !direct_mapping path in current patch.
Sounds right.
>
>> the question is now if you have
>> time to do that and the hardware to test that, or I'll have to finish
>> the work :)
>
> Sorry, for some reason part of this got lost in Evolution mail client.
>
> If possible, I do want to finish this work, and I am talking to IBM
> Virt people in order to get testing HW.
Even I struggle to find a powervm machine :)
>>
>>
>>> + else
>>> + pci->table_group->tables[1] = tbl;
>>
>>
>> What is this for?
>
> I was thinking of adding the older table to pci->table_group->tables[1]
> while keeping the newer table on pci->table_group->tables[0].
> This did work, but I think your suggestion may work better.
>
> Best regards,
> Leonardo Bras
>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v4 5/5] bus: Make remove callback return void
From: Geert Uytterhoeven @ 2021-07-14 8:44 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: nvdimm, linux-sh, Samuel Iglesias Gonsalvez, Jens Taprogge,
Ulf Hansson, Jaroslav Kysela, linux-fpga, Benjamin Tissoires,
Paul Mackerras, Srinivas Pandruvada, K. Y. Srinivasan,
Mike Christie, Wei Liu, Maxim Levitsky, Samuel Holland,
Rafael J . Wysocki, linux-acpi, linux-pci, xen-devel,
Tomas Winkler, Julien Grall, Ohad Ben-Cohen, Yufen Yu,
Alex Williamson, Alex Elder, linux-parisc, Finn Thain,
Geoff Levand, Greg Kroah-Hartman, linux-usb, Rafael J. Wysocki,
linux-kernel, linux-spi, Kai-Heng Feng, Sascha Hauer, Jon Mason,
linux-ntb, Wu Hao, David Woodhouse, Krzysztof Wilczyński,
Alexandre Belloni, Manohar Vanga, linux-wireless,
Dominik Brodowski, virtualization, James E.J. Bottomley,
target-devel, Srinivas Kandagatla, linux-i2c, linux-s390,
Stefano Stabellini, Stephen Hemminger, Yoshinori Sato, Jiri Slaby,
Helge Deller, Rafał Miłecki, YueHaibing,
industrypack-devel, linux-mips, Len Brown, Vasily Gorbik,
linux-arm-msm, linux-media, Maxime Ripard, William Breathitt Gray,
greybus-dev, linux-m68k, Florian Fainelli, Rikard Falkeborn,
Frank Li, Mark Gross, Boris Ostrovsky, Mika Westerberg,
linux-arm-kernel, Johannes Thumshirn, Mathieu Poirier,
Stephen Boyd, Cornelia Huck, Peter Oberparleiter, Wolfram Sang,
Joey Pabalan, Yehezkel Bernat, Pali Rohár, Bodo Stroesser,
Alison Schofield, Heikki Krogerus, Tyrel Datwyler,
Alexander Shishkin, Tom Rix, Jason Wang, SeongJae Park,
alsa-devel, platform-driver-x86, Bjorn Helgaas, netdev,
Vineeth Vijayan, Ira Weiny, Rob Herring, Lorenzo Pieralisi,
Dave Jiang, linux-staging, Dexuan Cui, Jernej Skrabec,
Kishon Vijay Abraham I, Christian Borntraeger, Chen-Yu Tsai,
linux-input, Allen Hubbe, Alex Dubov, Haiyang Zhang, Jiri Kosina,
Russell King (Oracle), Ben Widawsky, Harald Freudenberger,
linux-cxl, Michael Buesch, Dan Williams, Mauro Carvalho Chehab,
Cristian Marussi, Thomas Bogendoerfer, Martin K. Petersen,
Martyn Welch, Dmitry Torokhov, linux-mmc, linux-sunxi,
Stefan Richter, Sudeep Holla, David S. Miller, Sven Van Asbroeck,
Rich Felker, kvm, Michael S. Tsirkin, linux-remoteproc,
Bjorn Andersson, sparclinux, Kirti Wankhede, Andreas Noever,
linux-i3c, linux1394-devel, Lee Jones, Arnd Bergmann, linux-scsi,
Marc Zyngier, Russell King, Thorsten Scherer, Andy Gross,
linux-serial, Jakub Kicinski, linux-hyperv, Michael Jamet,
Heiko Carstens, Johan Hovold, Hans de Goede, Hannes Reinecke,
Juergen Gross, linuxppc-dev, Takashi Iwai, Alexandre Bounine,
Vinod Koul, Mark Brown, Vishal Verma, dmaengine, Moritz Fischer,
Johannes Berg, Johannes Thumshirn, Maximilian Luz
In-Reply-To: <20210713193522.1770306-6-u.kleine-koenig@pengutronix.de>
On Tue, Jul 13, 2021 at 9:35 PM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> The driver core ignores the return value of this callback because there
> is only little it can do when a device disappears.
>
> This is the final bit of a long lasting cleanup quest where several
> buses were converted to also return void from their remove callback.
> Additionally some resource leaks were fixed that were caused by drivers
> returning an error code in the expectation that the driver won't go
> away.
>
> With struct bus_type::remove returning void it's prevented that newly
> implemented buses return an ignored error code and so don't anticipate
> wrong expectations for driver authors.
> drivers/zorro/zorro-driver.c | 3 +--
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v4 5/5] bus: Make remove callback return void
From: Sudeep Holla @ 2021-07-14 10:25 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: nvdimm, linux-sh, Samuel Iglesias Gonsalvez, Jens Taprogge,
Ulf Hansson, Jaroslav Kysela, linux-fpga, Benjamin Tissoires,
Paul Mackerras, Srinivas Pandruvada, K. Y. Srinivasan,
Mike Christie, Wei Liu, Maxim Levitsky, Samuel Holland,
Rafael J . Wysocki, linux-acpi, Geert Uytterhoeven, linux-pci,
xen-devel, Tomas Winkler, Julien Grall, Ohad Ben-Cohen, Yufen Yu,
Alex Williamson, Alex Elder, linux-parisc, Finn Thain,
Geoff Levand, Greg Kroah-Hartman, linux-usb, Rafael J. Wysocki,
linux-kernel, linux-spi, Kai-Heng Feng, kernel, Jon Mason,
linux-ntb, Wu Hao, David Woodhouse, Krzysztof Wilczyński,
Alexandre Belloni, Manohar Vanga, linux-wireless,
Dominik Brodowski, virtualization, James E.J. Bottomley,
target-devel, Srinivas Kandagatla, linux-i2c, linux-s390,
Stefano Stabellini, Stephen Hemminger, Yoshinori Sato, Jiri Slaby,
Helge Deller, Rafał Miłecki, YueHaibing,
industrypack-devel, linux-mips, Len Brown, Vasily Gorbik,
linux-arm-msm, linux-media, Maxime Ripard, William Breathitt Gray,
greybus-dev, linux-m68k, Florian Fainelli, Rikard Falkeborn,
Frank Li, Mark Gross, Boris Ostrovsky, Mika Westerberg,
linux-arm-kernel, Johannes Thumshirn, Mathieu Poirier,
Stephen Boyd, Cornelia Huck, Peter Oberparleiter, Wolfram Sang,
Joey Pabalan, Yehezkel Bernat, Pali Rohár, Bodo Stroesser,
Alison Schofield, Heikki Krogerus, Tyrel Datwyler,
Alexander Shishkin, Tom Rix, Jason Wang, SeongJae Park,
alsa-devel, platform-driver-x86, Bjorn Helgaas, netdev,
Vineeth Vijayan, Ira Weiny, Rob Herring, Lorenzo Pieralisi,
Dave Jiang, linux-staging, Dexuan Cui, Jernej Skrabec,
Kishon Vijay Abraham I, Christian Borntraeger, Chen-Yu Tsai,
linux-input, Allen Hubbe, Alex Dubov, Haiyang Zhang, Jiri Kosina,
Russell King (Oracle), Ben Widawsky, Harald Freudenberger,
linux-cxl, Michael Buesch, Dan Williams, Mauro Carvalho Chehab,
Cristian Marussi, Thomas Bogendoerfer, Martin K. Petersen,
Martyn Welch, Dmitry Torokhov, linux-mmc, linux-sunxi,
Stefan Richter, Sudeep Holla, David S. Miller, Sven Van Asbroeck,
Rich Felker, kvm, Michael S. Tsirkin, linux-remoteproc,
Bjorn Andersson, sparclinux, Kirti Wankhede, Andreas Noever,
linux-i3c, linux1394-devel, Lee Jones, Arnd Bergmann, linux-scsi,
Marc Zyngier, Russell King, Thorsten Scherer, Andy Gross,
linux-serial, Jakub Kicinski, linux-hyperv, Michael Jamet,
Heiko Carstens, Johan Hovold, Hans de Goede, Hannes Reinecke,
Juergen Gross, linuxppc-dev, Takashi Iwai, Alexandre Bounine,
Vinod Koul, Mark Brown, Vishal Verma, dmaengine, Moritz Fischer,
Johannes Berg, Johannes Thumshirn, Maximilian Luz
In-Reply-To: <20210713193522.1770306-6-u.kleine-koenig@pengutronix.de>
On Tue, Jul 13, 2021 at 09:35:22PM +0200, Uwe Kleine-König wrote:
> The driver core ignores the return value of this callback because there
> is only little it can do when a device disappears.
>
> This is the final bit of a long lasting cleanup quest where several
> buses were converted to also return void from their remove callback.
> Additionally some resource leaks were fixed that were caused by drivers
> returning an error code in the expectation that the driver won't go
> away.
>
> With struct bus_type::remove returning void it's prevented that newly
> implemented buses return an ignored error code and so don't anticipate
> wrong expectations for driver authors.
>
[...]
> diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
> index 784cf0027da3..2682c3df651c 100644
> --- a/drivers/firmware/arm_scmi/bus.c
> +++ b/drivers/firmware/arm_scmi/bus.c
> @@ -116,15 +116,13 @@ static int scmi_dev_probe(struct device *dev)
> return scmi_drv->probe(scmi_dev);
> }
>
> -static int scmi_dev_remove(struct device *dev)
> +static void scmi_dev_remove(struct device *dev)
> {
> struct scmi_driver *scmi_drv = to_scmi_driver(dev->driver);
> struct scmi_device *scmi_dev = to_scmi_dev(dev);
>
> if (scmi_drv->remove)
> scmi_drv->remove(scmi_dev);
> -
> - return 0;
> }
>
> static struct bus_type scmi_bus_type = {
Acked-by: Sudeep Holla <sudeep.holla@arm.com>
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH v2] powerpc/rtas_flash: fix a potential buffer overflow
From: Yi Zhuang @ 2021-07-14 12:27 UTC (permalink / raw)
To: benh, paulus; +Cc: hegdevasant, linuxppc-dev, linux-kernel, zhuangyi1
Since snprintf() returns the possible output size instead of the
actual output size, the available flash_msg length returned by
get_validate_flash_msg may exceed the given buffer limit when
simple_read_from_buffer calls copy_to_user
Reported-by: kernel test robot <lkp@intel.com>
Fixes: a94a14720eaf5 powerpc/rtas_flash: Fix validate_flash buffer overflow issue
Signed-off-by: Yi Zhuang <zhuangyi1@huawei.com>
---
arch/powerpc/kernel/rtas_flash.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index a99179d83538..062f0724c2ff 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -470,9 +470,14 @@ static int get_validate_flash_msg(struct rtas_validate_flash_t *args_buf,
if (args_buf->status >= VALIDATE_TMP_UPDATE) {
n = sprintf(msg, "%d\n", args_buf->update_results);
if ((args_buf->update_results >= VALIDATE_CUR_UNKNOWN) ||
- (args_buf->update_results == VALIDATE_TMP_UPDATE))
+ (args_buf->update_results == VALIDATE_TMP_UPDATE)) {
n += snprintf(msg + n, msglen - n, "%s\n",
args_buf->buf);
+ if (n >= msglen) {
+ n = msglen;
+ printk(KERN_ERR "FLASH: msg too long.\n");
+ }
+ }
} else {
n = sprintf(msg, "%d\n", args_buf->status);
}
--
2.26.0.106.g9fadedd
^ permalink raw reply related
* Re: [RFC PATCH 10/43] powerpc/64s: Always set PMU control registers to frozen/disabled when not in use
From: Nicholas Piggin @ 2021-07-14 12:39 UTC (permalink / raw)
To: Athira Rajeev; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <1626057462.8m12ralsd6.astroid@bobo.none>
Excerpts from Nicholas Piggin's message of July 12, 2021 12:41 pm:
> Excerpts from Athira Rajeev's message of July 10, 2021 12:50 pm:
>>
>>
>>> On 22-Jun-2021, at 4:27 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
>>>
>>> KVM PMU management code looks for particular frozen/disabled bits in
>>> the PMU registers so it knows whether it must clear them when coming
>>> out of a guest or not. Setting this up helps KVM make these optimisations
>>> without getting confused. Longer term the better approach might be to
>>> move guest/host PMU switching to the perf subsystem.
>>>
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>> arch/powerpc/kernel/cpu_setup_power.c | 4 ++--
>>> arch/powerpc/kernel/dt_cpu_ftrs.c | 6 +++---
>>> arch/powerpc/kvm/book3s_hv.c | 5 +++++
>>> arch/powerpc/perf/core-book3s.c | 7 +++++++
>>> 4 files changed, 17 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kernel/cpu_setup_power.c b/arch/powerpc/kernel/cpu_setup_power.c
>>> index a29dc8326622..3dc61e203f37 100644
>>> --- a/arch/powerpc/kernel/cpu_setup_power.c
>>> +++ b/arch/powerpc/kernel/cpu_setup_power.c
>>> @@ -109,7 +109,7 @@ static void init_PMU_HV_ISA207(void)
>>> static void init_PMU(void)
>>> {
>>> mtspr(SPRN_MMCRA, 0);
>>> - mtspr(SPRN_MMCR0, 0);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>> mtspr(SPRN_MMCR1, 0);
>>> mtspr(SPRN_MMCR2, 0);
>>> }
>>> @@ -123,7 +123,7 @@ static void init_PMU_ISA31(void)
>>> {
>>> mtspr(SPRN_MMCR3, 0);
>>> mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>> - mtspr(SPRN_MMCR0, MMCR0_PMCCEXT);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>> }
>>>
>>> /*
>>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
>>> index 0a6b36b4bda8..06a089fbeaa7 100644
>>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>>> @@ -353,7 +353,7 @@ static void init_pmu_power8(void)
>>> }
>>>
>>> mtspr(SPRN_MMCRA, 0);
>>> - mtspr(SPRN_MMCR0, 0);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>> mtspr(SPRN_MMCR1, 0);
>>> mtspr(SPRN_MMCR2, 0);
>>> mtspr(SPRN_MMCRS, 0);
>>> @@ -392,7 +392,7 @@ static void init_pmu_power9(void)
>>> mtspr(SPRN_MMCRC, 0);
>>>
>>> mtspr(SPRN_MMCRA, 0);
>>> - mtspr(SPRN_MMCR0, 0);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>> mtspr(SPRN_MMCR1, 0);
>>> mtspr(SPRN_MMCR2, 0);
>>> }
>>> @@ -428,7 +428,7 @@ static void init_pmu_power10(void)
>>>
>>> mtspr(SPRN_MMCR3, 0);
>>> mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>> - mtspr(SPRN_MMCR0, MMCR0_PMCCEXT);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>> }
>>>
>>> static int __init feat_enable_pmu_power10(struct dt_cpu_feature *f)
>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>> index 1f30f98b09d1..f7349d150828 100644
>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>> @@ -2593,6 +2593,11 @@ static int kvmppc_core_vcpu_create_hv(struct kvm_vcpu *vcpu)
>>> #endif
>>> #endif
>>> vcpu->arch.mmcr[0] = MMCR0_FC;
>>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>>> + vcpu->arch.mmcr[0] |= MMCR0_PMCCEXT;
>>> + vcpu->arch.mmcra = MMCRA_BHRB_DISABLE;
>>> + }
>>> +
>>> vcpu->arch.ctrl = CTRL_RUNLATCH;
>>> /* default to host PVR, since we can't spoof it */
>>> kvmppc_set_pvr_hv(vcpu, mfspr(SPRN_PVR));
>>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>>> index 51622411a7cc..e33b29ec1a65 100644
>>> --- a/arch/powerpc/perf/core-book3s.c
>>> +++ b/arch/powerpc/perf/core-book3s.c
>>> @@ -1361,6 +1361,13 @@ static void power_pmu_enable(struct pmu *pmu)
>>> goto out;
>>>
>>> if (cpuhw->n_events == 0) {
>>> + if (cpu_has_feature(CPU_FTR_ARCH_31)) {
>>> + mtspr(SPRN_MMCRA, MMCRA_BHRB_DISABLE);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC | MMCR0_PMCCEXT);
>>> + } else {
>>> + mtspr(SPRN_MMCRA, 0);
>>> + mtspr(SPRN_MMCR0, MMCR0_FC);
>>> + }
>>
>>
>> Hi Nick,
>>
>> We are setting these bits in “power_pmu_disable” function. And disable will be called before any event gets deleted/stopped. Can you please help to understand why this is needed in power_pmu_enable path also ?
>
> I'll have to go back and check what I needed it for.
Okay, MMCRA is getting MMCRA_SDAR_MODE_DCACHE set on POWER9, by the looks.
That's not necessarily a problem, but KVM sets MMCRA to 0 to disable
SDAR updates. So KVM and perf don't agree on what the "correct" value
for disabled is. Which could be a problem with POWER10 not setting BHRB
disable before my series.
I'll get rid of this hunk for now, I expect things won't be exactly clean
or consistent until the KVM host PMU code is moved into perf/ though.
Thanks,
Nick
^ permalink raw reply
* [PATCH] powerpc/kexec: blacklist functions called in real mode for kprobe
From: Hari Bathini @ 2021-07-14 12:47 UTC (permalink / raw)
To: Michael Ellerman
Cc: Naveen N. Rao, linuxppc-dev, Mahesh J Salgaonkar, Sourabh Jain
As kprobe does not handle events happening in real mode, blacklist the
functions that only get called in real mode or in kexec sequence with
MMU turned off.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
arch/powerpc/kernel/head_64.S | 2 ++
arch/powerpc/kexec/core_64.c | 6 ++++--
arch/powerpc/mm/book3s64/hash_native.c | 2 +-
arch/powerpc/mm/book3s64/pgtable.c | 4 ++--
arch/powerpc/mm/book3s64/radix_pgtable.c | 3 ++-
arch/powerpc/platforms/ps3/htab.c | 3 ++-
arch/powerpc/platforms/ps3/mm.c | 8 ++++++--
arch/powerpc/platforms/pseries/lpar.c | 9 ++++++---
8 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 79930b0bc781..f17ae2083733 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -712,6 +712,8 @@ _GLOBAL(copy_and_flush)
isync
blr
+_ASM_NOKPROBE_SYMBOL(copy_and_flush); /* Called in real mode */
+
.align 8
copy_to_here:
diff --git a/arch/powerpc/kexec/core_64.c b/arch/powerpc/kexec/core_64.c
index 8a449b2d8715..84618d3c8013 100644
--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -72,7 +72,8 @@ int default_machine_kexec_prepare(struct kimage *image)
return 0;
}
-static void copy_segments(unsigned long ind)
+/* Called during kexec sequence with MMU off */
+static notrace void copy_segments(unsigned long ind)
{
unsigned long entry;
unsigned long *ptr;
@@ -105,7 +106,8 @@ static void copy_segments(unsigned long ind)
}
}
-void kexec_copy_flush(struct kimage *image)
+/* Called during kexec sequence with MMU off */
+notrace void kexec_copy_flush(struct kimage *image)
{
long i, nr_segments = image->nr_segments;
struct kexec_segment ranges[KEXEC_SEGMENT_MAX];
diff --git a/arch/powerpc/mm/book3s64/hash_native.c b/arch/powerpc/mm/book3s64/hash_native.c
index 52e170bd95ae..d8279bfe68ea 100644
--- a/arch/powerpc/mm/book3s64/hash_native.c
+++ b/arch/powerpc/mm/book3s64/hash_native.c
@@ -787,7 +787,7 @@ static void hpte_decode(struct hash_pte *hpte, unsigned long slot,
* TODO: add batching support when enabled. remember, no dynamic memory here,
* although there is the control page available...
*/
-static void native_hpte_clear(void)
+static notrace void native_hpte_clear(void)
{
unsigned long vpn = 0;
unsigned long slot, slots;
diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
index 9ffa65074cb0..300099de553b 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -172,8 +172,8 @@ pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
}
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
-/* For use by kexec */
-void mmu_cleanup_all(void)
+/* For use by kexec, called with MMU off */
+notrace void mmu_cleanup_all(void)
{
if (radix_enabled())
radix__mmu_cleanup_all();
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index e50ddf129c15..ae20add7954a 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -679,7 +679,8 @@ void radix__early_init_mmu_secondary(void)
mtspr(SPRN_UAMOR, 0);
}
-void radix__mmu_cleanup_all(void)
+/* Called during kexec sequence with MMU off */
+notrace void radix__mmu_cleanup_all(void)
{
unsigned long lpcr;
diff --git a/arch/powerpc/platforms/ps3/htab.c b/arch/powerpc/platforms/ps3/htab.c
index 7ddc7ec6a7c0..ef710a715903 100644
--- a/arch/powerpc/platforms/ps3/htab.c
+++ b/arch/powerpc/platforms/ps3/htab.c
@@ -169,7 +169,8 @@ static void ps3_hpte_invalidate(unsigned long slot, unsigned long vpn,
spin_unlock_irqrestore(&ps3_htab_lock, flags);
}
-static void ps3_hpte_clear(void)
+/* Called during kexec sequence with MMU off */
+static notrace void ps3_hpte_clear(void)
{
unsigned long hpte_count = (1UL << ppc64_pft_size) >> 4;
u64 i;
diff --git a/arch/powerpc/platforms/ps3/mm.c b/arch/powerpc/platforms/ps3/mm.c
index a81eac35d900..9c44f335c0b9 100644
--- a/arch/powerpc/platforms/ps3/mm.c
+++ b/arch/powerpc/platforms/ps3/mm.c
@@ -195,9 +195,11 @@ void __init ps3_mm_vas_create(unsigned long* htab_size)
/**
* ps3_mm_vas_destroy -
+ *
+ * called during kexec sequence with MMU off.
*/
-void ps3_mm_vas_destroy(void)
+notrace void ps3_mm_vas_destroy(void)
{
int result;
@@ -1243,9 +1245,11 @@ void __init ps3_mm_init(void)
/**
* ps3_mm_shutdown - final cleanup of address space
+ *
+ * called during kexec sequence with MMU off.
*/
-void ps3_mm_shutdown(void)
+notrace void ps3_mm_shutdown(void)
{
ps3_mm_region_destroy(&map.r1);
}
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index dab356e3ff87..869ef638698a 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -801,7 +801,8 @@ static long pSeries_lpar_hpte_remove(unsigned long hpte_group)
return -1;
}
-static void manual_hpte_clear_all(void)
+/* Called during kexec sequence with MMU off */
+static notrace void manual_hpte_clear_all(void)
{
unsigned long size_bytes = 1UL << ppc64_pft_size;
unsigned long hpte_count = size_bytes >> 4;
@@ -834,7 +835,8 @@ static void manual_hpte_clear_all(void)
}
}
-static int hcall_hpte_clear_all(void)
+/* Called during kexec sequence with MMU off */
+static notrace int hcall_hpte_clear_all(void)
{
int rc;
@@ -845,7 +847,8 @@ static int hcall_hpte_clear_all(void)
return rc;
}
-static void pseries_hpte_clear_all(void)
+/* Called during kexec sequence with MMU off */
+static notrace void pseries_hpte_clear_all(void)
{
int rc;
^ permalink raw reply related
* Re: [PATCH v2] powerpc/rtas_flash: fix a potential buffer overflow
From: Christophe Leroy @ 2021-07-14 16:52 UTC (permalink / raw)
To: Yi Zhuang; +Cc: hegdevasant, paulus, linuxppc-dev, linux-kernel
In-Reply-To: <20210714122753.76021-1-zhuangyi1@huawei.com>
Yi Zhuang <zhuangyi1@huawei.com> a écrit :
> Since snprintf() returns the possible output size instead of the
> actual output size, the available flash_msg length returned by
> get_validate_flash_msg may exceed the given buffer limit when
> simple_read_from_buffer calls copy_to_user
>
> Reported-by: kernel test robot <lkp@intel.com>
> Fixes: a94a14720eaf5 powerpc/rtas_flash: Fix validate_flash buffer
> overflow issue
> Signed-off-by: Yi Zhuang <zhuangyi1@huawei.com>
> ---
> arch/powerpc/kernel/rtas_flash.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/rtas_flash.c
> b/arch/powerpc/kernel/rtas_flash.c
> index a99179d83538..062f0724c2ff 100644
> --- a/arch/powerpc/kernel/rtas_flash.c
> +++ b/arch/powerpc/kernel/rtas_flash.c
> @@ -470,9 +470,14 @@ static int get_validate_flash_msg(struct
> rtas_validate_flash_t *args_buf,
> if (args_buf->status >= VALIDATE_TMP_UPDATE) {
> n = sprintf(msg, "%d\n", args_buf->update_results);
> if ((args_buf->update_results >= VALIDATE_CUR_UNKNOWN) ||
> - (args_buf->update_results == VALIDATE_TMP_UPDATE))
> + (args_buf->update_results == VALIDATE_TMP_UPDATE)) {
> n += snprintf(msg + n, msglen - n, "%s\n",
> args_buf->buf);
> + if (n >= msglen) {
n cannot be greater than msglen
> + n = msglen;
> + printk(KERN_ERR "FLASH: msg too long.\n");
> + }
> + }
> } else {
> n = sprintf(msg, "%d\n", args_buf->status);
> }
> --
> 2.26.0.106.g9fadedd
^ permalink raw reply
* [Bug 213733] New: Kernel NULL pointer dereference on read (Oops: Kernel access of bad area, sig: 7 [#1]) at systemctl poweroff
From: bugzilla-daemon @ 2021-07-14 17:08 UTC (permalink / raw)
To: linuxppc-dev
https://bugzilla.kernel.org/show_bug.cgi?id=213733
Bug ID: 213733
Summary: Kernel NULL pointer dereference on read (Oops: Kernel
access of bad area, sig: 7 [#1]) at systemctl poweroff
Product: Platform Specific/Hardware
Version: 2.5
Kernel Version: 5.14-rc1
Hardware: PPC-64
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: PPC-64
Assignee: platform_ppc-64@kernel-bugs.osdl.org
Reporter: erhard_f@mailbox.org
Regression: No
Created attachment 297857
--> https://bugzilla.kernel.org/attachment.cgi?id=297857&action=edit
dmesg (kernel 5.14-rc1, Talos II)
My Talos II run fine for a few hours building stuff but at system shutdown
(systemctl poweroff) I got this:
[...]
BUG: Kernel NULL pointer dereference on read at 0x00000000
Faulting instruction address: 0xc00000000034396c
Oops: Kernel access of bad area, sig: 7 [#1]
BE PAGE_SIZE=4K MMU=Radix SMP NR_CPUS=192 DEBUG_PAGEALLOC NUMA PowerNV
Modules linked in: auth_rpcgss nfsv4 dns_resolver nfs lockd grace sunrpc rfkill
ecb xts ctr cbc aes_generic libaes ibmpowernv evdev radeon snd_hda_codec_hdmi
snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep ghash_generic
snd_hda_core drm_ttm_helper xhci_pci ofpart snd_pcm vmx_crypto ttm gf128mul
powernv_flash xhci_hcd mtd i2c_algo_bit snd_timer opal_prd hwmon drm_kms_helper
usbcore cfbfillrect cfbcopyarea cfbimgblt sysimgblt snd syscopyarea sysfillrect
fb_sys_fops usb_common soundcore at24 regmap_i2c zram zsmalloc powernv_cpufreq
drm fuse drm_panel_orientation_quirks backlight configfs
CPU: 26 PID: 345930 Comm: kworker/u66:5 Not tainted 5.14.0-rc1-TalosII #2
Workqueue: events_unbound .cleanup_offline_cgwbs_workfn
NIP: c00000000034396c LR: c000000000343850 CTR: 0000000000000000
REGS: c00020016bf9f7d0 TRAP: 0300 Not tainted (5.14.0-rc1-TalosII)
MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI> CR: 44002228 XER: 00000004
CFAR: c000000000343864 DAR: 0000000000000000 DSISR: 00080000 IRQMASK: 1
GPR00: c000000000343848 c00020016bf9fa70 c0000000012d6100 0000000000000001
GPR04: c000200183630ac0 00000000ffffffff 0000000090163e29 0040900000000000
GPR08: 0000000000000000 0000000000000003 0000000000000000 c00000000168c488
GPR12: 0000000044002228 c0002007ff7f4c00 c000000000115e20 c000200002950340
GPR16: 0000000000000000 0000000000000001 c00000000112ef30 c000000000ea2db8
GPR20: c000000000ea2d68 c000000000ea2d98 0000000000000001 c0000000011c6352
GPR24: 0000000000000001 c000200183630080 c00000000114cf28 c00000000114ced8
GPR28: c00020016bf9faf8 c00000000114cde8 c00020000229a000 c00020000229a510
NIP [c00000000034396c] .cleanup_offline_cgwbs_workfn+0x3ac/0x410
LR [c000000000343850] .cleanup_offline_cgwbs_workfn+0x290/0x410
Call Trace:
[c00020016bf9fa70] [c000000000343848] .cleanup_offline_cgwbs_workfn+0x288/0x410
(unreliable)
[c00020016bf9fb90] [c00000000010871c] .process_one_work+0x2dc/0x7d0
[c00020016bf9fc70] [c000000000108ca8] .worker_thread+0x98/0x500
[c00020016bf9fd50] [c000000000115fa8] .kthread+0x188/0x190
[c00020016bf9fe10] [c00000000000cef8] .ret_from_kernel_thread+0x58/0x60
Instruction dump:
7e85a378 388002b0 7ea3ab78 9ad70002 4be3ff21 60000000 e93fff08 712a0003
4182ff0c e95fff10 39000000 7c0004ac <7d2050a8> 7c294000 41820018 7d384a14
---[ end trace d475291d44c4d324 ]---
note: kworker/u66:5[345930] exited with preempt_count 2
watchdog: CPU 16 self-detected hard LOCKUP @ .do_raw_spin_lock+0x90/0x1d0
watchdog: CPU 16 TB:5873709870913, last heartbeat TB:5867694041597 (11749ms
ago)
Modules linked in: auth_rpcgss nfsv4 dns_resolver nfs lockd grace sunrpc rfkill
ecb xts ctr cbc aes_generic libaes ibmpowernv evdev radeon snd_hda_codec_hdmi
snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep ghash_generic
snd_hda_core drm_ttm_helper xhci_pci ofpart snd_pcm vmx_crypto ttm gf128mul
powernv_flash xhci_hcd mtd i2c_algo_bit snd_timer opal_prd hwmon drm_kms_helper
usbcore cfbfillrect cfbcopyarea cfbimgblt sysimgblt snd syscopyarea sysfillrect
fb_sys_fops usb_common soundcore at24 regmap_i2c zram zsmalloc powernv_cpufreq
drm fuse drm_panel_orientation_quirks backlight configfs
irq event stamp: 3697758
hardirqs last enabled at (3697757): [<c0000000003d8d84>]
.__slab_free+0x3b4/0x5f0
hardirqs last disabled at (3697758): [<c000000000c7e228>]
._raw_spin_lock_irq+0x88/0xa0
softirqs last enabled at (3697726): [<c000000000342d1c>]
.wb_shutdown+0x5c/0x140
softirqs last disabled at (3697724): [<c000000000342ce0>]
.wb_shutdown+0x20/0x140
CPU: 16 PID: 292187 Comm: kworker/16:0 Tainted: G D
5.14.0-rc1-TalosII #2
Workqueue: cgwb_release .cgwb_release_workfn
NIP: c000000000185e90 LR: c000000000c7e204 CTR: 0000000000000000
REGS: c0002007ff667d60 TRAP: 0900 Tainted: G D
(5.14.0-rc1-TalosII)
MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI> CR: 44002228 XER: 20040000
CFAR: c000000000185e9c IRQMASK: 1
GPR00: c000000000c7e1f8 c0002000e15cf9f0 c0000000012d6100 c00000000114ced8
GPR04: c0002000067b8a98 00000000ffffffff 00000000d7d91060 0e98300000000000
GPR08: ffffffff62ff980e 000000008000001a 0000000080000010 c00000000168c488
GPR12: 0000000044002222 c0002007ff7ffc00 c000000000115e20 c000200002950640
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 c00000000133fa08 c0000000010f0840
GPR24: c0002000022d52d4 0000000000000000 c0000000012e8ce0 0000000000000000
GPR28: c00020000229a000 0000000000000001 c00020000229a520 c00000000114ced8
NIP [c000000000185e90] .do_raw_spin_lock+0x90/0x1d0
LR [c000000000c7e204] ._raw_spin_lock_irq+0x64/0xa0
Call Trace:
[c0002000e15cf9f0] [c000000000c7d994] ._raw_spin_unlock_irqrestore+0x84/0xd0
(unreliable)
[c0002000e15cfa70] [c000000000c7e1f8] ._raw_spin_lock_irq+0x58/0xa0
[c0002000e15cfb00] [c000000000344c14] .cgwb_release_workfn+0xd4/0x200
[c0002000e15cfb90] [c00000000010871c] .process_one_work+0x2dc/0x7d0
[c0002000e15cfc70] [c000000000108ca8] .worker_thread+0x98/0x500
[c0002000e15cfd50] [c000000000115fa8] .kthread+0x188/0x190
[c0002000e15cfe10] [c00000000000cef8] .ret_from_kernel_thread+0x58/0x60
Instruction dump:
40820030 a14d0378 a0ed0002 38210080 e90d0128 394a0001 b14d0378 90ff0008
f91f0010 ebe1fff8 4e800020 60000000 <7c210b78> 813f0000 2c290000 4082fff4
Kernel panic - not syncing: Hard LOCKUP
watchdog: CPU 28 self-detected hard LOCKUP @ .do_raw_spin_lock+0x90/0x1d0
watchdog: CPU 28 TB:5873733764122, last heartbeat TB:5867705991531 (11772ms
ago)
Modules linked in: auth_rpcgss nfsv4 dns_resolver nfs lockd grace sunrpc rfkill
ecb xts ctr cbc aes_generic libaes ibmpowernv evdev radeon snd_hda_codec_hdmi
snd_hda_intel snd_intel_dspcfg snd_hda_codec snd_hwdep ghash_generic
snd_hda_core drm_ttm_helper xhci_pci ofpart snd_pcm vmx_crypto ttm gf128mul
powernv_flash xhci_hcd mtd i2c_algo_bit snd_timer opal_prd hwmon drm_kms_helper
usbcore cfbfillrect cfbcopyarea cfbimgblt sysimgblt snd syscopyarea sysfillrect
fb_sys_fops usb_common soundcore at24 regmap_i2c zram zsmalloc powernv_cpufreq
drm fuse drm_panel_orientation_quirks backlight configfs
irq event stamp: 0
hardirqs last enabled at (0): [<0000000000000000>] 0x0
hardirqs last disabled at (0): [<c0000000000ce3c4>] .copy_process+0x1134/0x3fd0
softirqs last enabled at (0): [<c0000000000ce3c4>] .copy_process+0x1134/0x3fd0
softirqs last disabled at (0): [<0000000000000000>] 0x0
CPU: 28 PID: 425922 Comm: systemd-update- Tainted: G D
5.14.0-rc1-TalosII #2
NIP: c000000000185e90 LR: c000000000c7e3f8 CTR: 0000000000000000
REGS: c0002007ff5d7d60 TRAP: 0900 Tainted: G D
(5.14.0-rc1-TalosII)
MSR: 9000000000009032 <SF,HV,EE,ME,IR,DR,RI> CR: 44002482 XER: 2004008c
CFAR: c000000000185e9c IRQMASK: 1
GPR00: c000000000c7e3ec c00020006d5ef610 c0000000012d6100 c00000000114ced8
GPR04: 0000000000000000 0000000000000000 0000000000000000 0000000000000001
GPR08: 0000000000000000 000000008000001a 000000008000001c fffffffffffffffd
GPR12: 0000000024002482 c0002007ff7f4400 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000180 00000000003e0100 c000000010f78000
GPR20: c00020000265c380 0000000000000000 c0000000020a37b8 0000000000000000
GPR24: c000200002ba15d8 c00000000114ced8 0000000000000cc0 c000200002ba1000
GPR28: 0000000000000000 c00020000265c380 0000000000000000 c00000000114ced8
NIP [c000000000185e90] .do_raw_spin_lock+0x90/0x1d0
LR [c000000000c7e3f8] ._raw_spin_lock_irqsave+0x68/0xb0
Call Trace:
[c00020006d5ef610] [c00000000112ef30] rcu_lock_map+0x0/0x28 (unreliable)
[c00020006d5ef690] [c000000000c7e3ec] ._raw_spin_lock_irqsave+0x5c/0xb0
[c00020006d5ef720] [c000000000344e2c] .wb_get_create+0xec/0x8d0
[c00020006d5ef820] [c00000000031848c]
.balance_dirty_pages_ratelimited+0x2ec/0x880
[c00020006d5ef8e0] [c00000000064c798] .btrfs_buffered_write+0x548/0x740
[c00020006d5efa40] [c00000000064cb9c] .btrfs_file_write_iter+0x20c/0x4f0
[c00020006d5efb10] [c000000000419254] .new_sync_write+0x124/0x1c0
[c00020006d5efc00] [c00000000041b7e4] .vfs_write+0x1c4/0x230
[c00020006d5efca0] [c00000000041ba04] .ksys_write+0x74/0x130
[c00020006d5efd40] [c000000000033ea0] .system_call_exception+0x1c0/0x420
[c00020006d5efe10] [c00000000000c070] system_call_vectored_common+0xf0/0x280
--- interrupt: 3000 at 0x3fff9ea5a73c
NIP: 00003fff9ea5a73c LR: 0000000000000000 CTR: 0000000000000000
REGS: c00020006d5efe80 TRAP: 3000 Tainted: G D
(5.14.0-rc1-TalosII)
MSR: 900000000280f032 <SF,HV,VEC,VSX,EE,PR,FP,ME,IR,DR,RI> CR: 44000482 XER:
00000000
IRQMASK: 0
GPR00: 0000000000000004 00003fffedcba9a0 00003fff9eb44300 0000000000000005
GPR04: 00003fffedcbac68 0000000000000180 00003fffedcba990 0000000000000002
GPR08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR12: 0000000000000000 00003fff9ef78810 00003ffff543cda8 0000000000000000
GPR16: 0000000000000000 0000000000000000 00003ffff543ca00 00000001525b4260
GPR20: 00003ffff543ca18 0000000000000001 0000000000000000 0000000000000001
GPR24: 0000000000000000 0000000000000000 0000000000000000 0000000000000002
GPR28: 0000000000000000 00003fffedcbac68 00000000003e0100 0000000000000005
NIP [00003fff9ea5a73c] 0x3fff9ea5a73c
LR [0000000000000000] 0x0
--- interrupt: 3000
Instruction dump:
40820030 a14d0378 a0ed0002 38210080 e90d0128 394a0001 b14d0378 90ff0008
f91f0010 ebe1fff8 4e800020 60000000 <7c210b78> 813f0000 2c290000 4082fff4
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 213733] Kernel NULL pointer dereference on read (Oops: Kernel access of bad area, sig: 7 [#1]) at systemctl poweroff
From: bugzilla-daemon @ 2021-07-14 17:11 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-213733-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=213733
--- Comment #1 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 297859
--> https://bugzilla.kernel.org/attachment.cgi?id=297859&action=edit
kernel .config (kernel 5.14-rc1, Talos II)
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v4 07/11] powerpc/pseries/iommu: Reorganize iommu_table_setparms*() with new helper
From: Leonardo Brás @ 2021-07-14 19:02 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy, Nicolin Chen,
Niklas Schnelle
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <88e3b97e-201d-0782-0e95-8e3d2d850a38@ozlabs.ru>
On Wed, 2021-07-14 at 18:32 +1000, Alexey Kardashevskiy wrote:
>
>
> On 13/07/2021 14:47, Leonardo Brás wrote:
> > Hello Alexey,
> >
> > On Fri, 2021-06-18 at 19:26 -0300, Leonardo Brás wrote:
> > > >
> > > > > + unsigned long liobn,
> > > > > unsigned long win_addr,
> > > > > + unsigned long
> > > > > window_size,
> > > > > unsigned long page_shift,
> > > > > + unsigned long base,
> > > > > struct
> > > > > iommu_table_ops *table_ops)
> > > >
> > > >
> > > > iommu_table_setparms() rather than passing 0 around.
> > > >
> > > > The same comment about "liobn" - set it in
> > > > iommu_table_setparms_lpar().
> > > > The reviewer will see what field atters in what situation imho.
> > > >
> > >
> > > The idea here was to keep all tbl parameters setting in
> > > _iommu_table_setparms (or iommu_table_setparms_common).
> > >
> > > I understand the idea that each one of those is optional in the
> > > other
> > > case, but should we keep whatever value is present in the other
> > > variable (not zeroing the other variable), or do someting like:
> > >
> > > tbl->it_index = 0;
> > > tbl->it_base = basep;
> > > (in iommu_table_setparms)
> > >
> > > tbl->it_index = liobn;
> > > tbl->it_base = 0;
> > > (in iommu_table_setparms_lpar)
> > >
> >
> > This one is supposed to be a question, but I missed the question
> > mark.
> > Sorry about that.
>
> Ah ok :)
>
> > I would like to get your opinion in this :)
>
> Besides making the "base" parameter a pointer, I really do not have
> strong preference, just make it not hurting eyes of a reader, that's
> all :)
Ok, done :)
> imho in general, rather than answering 5 weeks later, it is more
> productive to address whatever comments were made, add comments (in
> the
> code or commit logs) why you are sticking to your initial approach,
> rebase and repost the whole thing. Thanks,
Thanks for the tip, and for the reviewing :)
Best regards,
Leonardo Bras
^ permalink raw reply
* [Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
From: bugzilla-daemon @ 2021-07-14 19:03 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-210749-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210749
--- Comment #9 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 297867
--> https://bugzilla.kernel.org/attachment.cgi?id=297867&action=edit
dmesg (kernel 5.14-rc1 w. 61f764c307f6, 4e302c3b568e reverted, Talos II)
Yes, I can confirm reverting 61f764c307f6 and 4e302c3b568e works. Tested on
kernel 5.14-rc1.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* [Bug 210749] sysfs: cannot create duplicate filename '/bus/nvmem/devices/module-vpd'
From: bugzilla-daemon @ 2021-07-14 19:03 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-210749-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=210749
Erhard F. (erhard_f@mailbox.org) changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #295621|0 |1
is obsolete| |
--- Comment #10 from Erhard F. (erhard_f@mailbox.org) ---
Created attachment 297869
--> https://bugzilla.kernel.org/attachment.cgi?id=297869&action=edit
kernel .config (kernel 5.14-rc1, Talos II)
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH v4 10/11] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Leonardo Brás @ 2021-07-14 19:25 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy, Nicolin Chen,
Niklas Schnelle
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <2e88a275-3379-6f14-3c93-d2919bee2142@ozlabs.ru>
On Wed, 2021-07-14 at 18:38 +1000, Alexey Kardashevskiy wrote:
> > for (i = 0; i <
> > > > ARRAY_SIZE(pci->phb->mem_resources);
> > > > i++) {
> > > > + const unsigned long mask =
> > > > IORESOURCE_MEM_64
> > > > > IORESOURCE_MEM;
> > > > +
> > > > + /* Look for MMIO32 */
> > > > + if ((pci->phb->mem_resources[i].flags &
> > > > mask)
> > > > == IORESOURCE_MEM)
> > > > + break;
> > >
> > > What if there is no IORESOURCE_MEM? pci->phb-
> > > >mem_resources[i].start
> > > below will have garbage.
> >
> >
> >
> > Yeah, that makes sense. I will add this lines after 'for':
> >
> > if (i == ARRAY_SIZE(pci->phb->mem_resources)) {
> > iommu_tce_table_put(newtbl);
> > goto out_del_list;
> > }
> >
> > What do you think?
>
>
> Move this and that "for" before iommu_pseries_alloc_table() so you
> won't
> need to free if there is no IORESOURCE_MEM.
Done!
>
>
> >
> >
> > >
> > >
> > > > + }
> > > > +
> > > > + _iommu_table_setparms(newtbl, pci->phb->bus-
> > > > >number,
> > > > create.liobn, win_addr,
> > > > + 1UL << len, page_shift,
> > > > 0,
> > > > &iommu_table_lpar_multi_ops);
> > > > + iommu_init_table(newtbl, pci->phb->node, pci-
> > > > >phb-
> > > > > mem_resources[i].start,
> > > > + pci->phb-
> > > > >mem_resources[i].end);
> > > > +
> > > > + if (default_win_removed)
> > > > + iommu_tce_table_put(tbl);
> > >
> > >
> > > iommu_tce_table_put() should have been called when the window was
> > > removed.
> > >
> > > Also after some thinking - what happens if there were 2 devices
> > > in the
> > > PE and one requested 64bit DMA? This will only update
> > > set_iommu_table_base() for the 64bit one but not for the other.
> > >
> > > I think the right thing to do is:
> > >
> > > 1. check if table[0] is in use, if yes => fail (which this does
> > > already)
> > >
> > > 2. remove default dma window but keep the iommu_table struct with
> > > one
> > > change - set it_size to 0 (and free it_map) so the 32bit device
> > > won't
> > > look at a stale structure and think there is some window
> > > (imaginery
> > > situation for phyp but easy to recreate in qemu).
> > >
> > > 3. use table[1] for newly created indirect DDW window.
> > >
> > > 4. change get_iommu_table_base() to return a usable table (or may
> > > be
> > > not
> > > needed?).
> > >
> > > If this sounds reasonable (does it?),
> >
> > Looks ok, I will try your suggestion.
> > I was not aware of how pci->table_group->tables[] worked, so I
> > replaced
> > pci->table_group->tables[0] with the new tbl, while moving the
> > older in
> > pci->table_group->tables[1].
>
>
> pci->table_group->tables[0] is window#0 at @0.
> pci->table_group->tables[1] is window#1 at 0x0800.0000.0000.0000.
> That
> is all :)
>
> pseries does not use tables[1] but powernv does (by VFIO only
> though).
Thanks! This helped a lot!
>
>
> > (4) get_iommu_table_base() does not seem to need update, as it
> > returns
> > the tlb set by set_iommu_table_base() which is already called in
> > the
> > !direct_mapping path in current patch.
>
> Sounds right.
>
> >
> > > the question is now if you have
> > > time to do that and the hardware to test that, or I'll have to
> > > finish
> > > the work :)
> >
> > Sorry, for some reason part of this got lost in Evolution mail
> > client.
> >
> > If possible, I do want to finish this work, and I am talking to IBM
> > Virt people in order to get testing HW.
>
>
> Even I struggle to find a powervm machine :)
>
> > >
> > >
> > > > + else
> > > > + pci->table_group->tables[1] = tbl;
> > >
> > >
> > > What is this for?
> >
> > I was thinking of adding the older table to pci->table_group-
> > >tables[1]
> > while keeping the newer table on pci->table_group->tables[0].
> > This did work, but I think your suggestion may work better.
> >
> > Best regards,
> > Leonardo Bras
> >
> >
>
Thanks a lot for reviewing Alexey!
I will send a v5 soon.
Best regards,
Leonardo Bras
^ permalink raw reply
* Re: [PATCH v1 2/4] mm/memory_hotplug: remove nid parameter from arch_remove_memory()
From: Pankaj Gupta @ 2021-07-14 20:47 UTC (permalink / raw)
To: David Hildenbrand
Cc: Michel Lespinasse, Kefeng Wang, Rich Felker, linux-ia64, Wei Yang,
Michael S. Tsirkin, Peter Zijlstra, Catalin Marinas, Jason Wang,
Dave Hansen, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
Michal Hocko, Linux MM, Paul Mackerras, H. Peter Anvin,
Will Deacon, Ard Biesheuvel, linux-s390, Laurent Dufour,
Dave Jiang, Baoquan He, Christian Borntraeger, linux-sh,
Rafael J. Wysocki, linux-acpi, Ingo Molnar, linux-arm-kernel,
Len Brown, Nathan Lynch, Pavel Tatashin, Vasily Gorbik,
Anshuman Khandual, Heiko Carstens, Nicholas Piggin, Vishal Verma,
Borislav Petkov, Sergei Trofimovich, Andy Lutomirski, Jia He,
Dan Williams, virtualization, Vitaly Kuznetsov, Vlastimil Babka,
Oscar Salvador, Christophe Leroy, Yoshinori Sato, Pierre Morel,
Scott Cheloha, Rafael J. Wysocki, LKML, Thomas Gleixner,
Thiago Jung Bauermann, Aneesh Kumar K.V, Joe Perches,
Andrew Morton, linuxppc-dev, Mike Rapoport
In-Reply-To: <20210712124052.26491-3-david@redhat.com>
> The parameter is unused, let's remove it.
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> Acked-by: Heiko Carstens <hca@linux.ibm.com> (s390)
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Cc: Rich Felker <dalias@libc.org>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: x86@kernel.org
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Laurent Dufour <ldufour@linux.ibm.com>
> Cc: Sergei Trofimovich <slyfox@gentoo.org>
> Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
> Cc: Michel Lespinasse <michel@lespinasse.org>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
> Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Cc: Jia He <justin.he@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-ia64@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-s390@vger.kernel.org
> Cc: linux-sh@vger.kernel.org
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> arch/arm64/mm/mmu.c | 3 +--
> arch/ia64/mm/init.c | 3 +--
> arch/powerpc/mm/mem.c | 3 +--
> arch/s390/mm/init.c | 3 +--
> arch/sh/mm/init.c | 3 +--
> arch/x86/mm/init_32.c | 3 +--
> arch/x86/mm/init_64.c | 3 +--
> include/linux/memory_hotplug.h | 3 +--
> mm/memory_hotplug.c | 4 ++--
> mm/memremap.c | 5 +----
> 10 files changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index d74586508448..af8ab553a268 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1506,8 +1506,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return ret;
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 064a967a7b6e..5c6da8d83c1a 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -484,8 +484,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return ret;
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index ad198b439222..c3c4e31462ec 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -119,8 +119,7 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
> return rc;
> }
>
> -void __ref arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void __ref arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index 8ac710de1ab1..d85bd7f5d8dc 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -306,8 +306,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return rc;
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index ce26c7f8950a..506784702430 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -414,8 +414,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return ret;
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = PFN_DOWN(start);
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index 74b78840182d..bd90b8fe81e4 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -801,8 +801,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
> return __add_pages(nid, start_pfn, nr_pages, params);
> }
>
> -void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index ddeaba947eb3..a6e11763763f 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1255,8 +1255,7 @@ kernel_physical_mapping_remove(unsigned long start, unsigned long end)
> remove_pagetable(start, end, true, NULL);
> }
>
> -void __ref arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap)
> +void __ref arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap)
> {
> unsigned long start_pfn = start >> PAGE_SHIFT;
> unsigned long nr_pages = size >> PAGE_SHIFT;
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index d01b504ce06f..010a192298b5 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -130,8 +130,7 @@ static inline bool movable_node_is_enabled(void)
> return movable_node_enabled;
> }
>
> -extern void arch_remove_memory(int nid, u64 start, u64 size,
> - struct vmem_altmap *altmap);
> +extern void arch_remove_memory(u64 start, u64 size, struct vmem_altmap *altmap);
> extern void __remove_pages(unsigned long start_pfn, unsigned long nr_pages,
> struct vmem_altmap *altmap);
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 93b3abaf9828..f2a9af3af184 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1106,7 +1106,7 @@ int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
> /* create memory block devices after memory was added */
> ret = create_memory_block_devices(start, size, mhp_altmap.alloc);
> if (ret) {
> - arch_remove_memory(nid, start, size, NULL);
> + arch_remove_memory(start, size, NULL);
> goto error;
> }
>
> @@ -1892,7 +1892,7 @@ static int __ref try_remove_memory(int nid, u64 start, u64 size)
>
> mem_hotplug_begin();
>
> - arch_remove_memory(nid, start, size, altmap);
> + arch_remove_memory(start, size, altmap);
>
> if (IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK)) {
> memblock_free(start, size);
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 15a074ffb8d7..ed593bf87109 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -140,14 +140,11 @@ static void pageunmap_range(struct dev_pagemap *pgmap, int range_id)
> {
> struct range *range = &pgmap->ranges[range_id];
> struct page *first_page;
> - int nid;
>
> /* make sure to access a memmap that was actually initialized */
> first_page = pfn_to_page(pfn_first(pgmap, range_id));
>
> /* pages are dead and unused, undo the arch mapping */
> - nid = page_to_nid(first_page);
> -
> mem_hotplug_begin();
> remove_pfn_range_from_zone(page_zone(first_page), PHYS_PFN(range->start),
> PHYS_PFN(range_len(range)));
> @@ -155,7 +152,7 @@ static void pageunmap_range(struct dev_pagemap *pgmap, int range_id)
> __remove_pages(PHYS_PFN(range->start),
> PHYS_PFN(range_len(range)), NULL);
> } else {
> - arch_remove_memory(nid, range->start, range_len(range),
> + arch_remove_memory(range->start, range_len(range),
> pgmap_altmap(pgmap));
> kasan_remove_zero_shadow(__va(range->start), range_len(range));
> }
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
^ permalink raw reply
* Re: [PATCH] soc: fsl: qe: convert QE interrupt controller to platform_device
From: Li Yang @ 2021-07-14 22:29 UTC (permalink / raw)
To: Maxim Kochetkov
Cc: saravanak, Greg Kroah-Hartman, lkml, linuxppc-dev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Zhao Qiang
In-Reply-To: <20210705111250.1513634-1-fido_max@inbox.ru>
On Mon, Jul 5, 2021 at 6:12 AM Maxim Kochetkov <fido_max@inbox.ru> wrote:
>
> Since 5.13 QE's ucc nodes can't get interrupts from devicetree:
>
> ucc@2000 {
> cell-index = <1>;
> reg = <0x2000 0x200>;
> interrupts = <32>;
> interrupt-parent = <&qeic>;
> };
>
> Now fw_devlink expects driver to create and probe a struct device
> for interrupt controller.
>
> So lets convert this driver to simple platform_device with probe().
>
> [1] - https://lore.kernel.org/lkml/CAGETcx9PiX==mLxB9PO8Myyk6u2vhPVwTMsA5NkD-ywH5xhusw@mail.gmail.com
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
> Fixes: ea718c699055 ("Revert "Revert "driver core: Set fw_devlink=on by default""")
> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
> ---
> drivers/soc/fsl/qe/qe_ic.c | 38 +++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
> index 3f711c1a0996..03d291376895 100644
> --- a/drivers/soc/fsl/qe/qe_ic.c
> +++ b/drivers/soc/fsl/qe/qe_ic.c
> @@ -23,6 +23,7 @@
> #include <linux/signal.h>
> #include <linux/device.h>
> #include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> #include <asm/irq.h>
> #include <asm/io.h>
> #include <soc/fsl/qe/qe.h>
> @@ -404,27 +405,28 @@ static void qe_ic_cascade_muxed_mpic(struct irq_desc *desc)
> chip->irq_eoi(&desc->irq_data);
> }
>
> -static void __init qe_ic_init(struct device_node *node)
> +static int qe_ic_init(struct platform_device *pdev)
> {
> void (*low_handler)(struct irq_desc *desc);
> void (*high_handler)(struct irq_desc *desc);
> struct qe_ic *qe_ic;
> struct resource res;
> + struct device_node *node = pdev->dev.of_node;
> u32 ret;
>
> ret = of_address_to_resource(node, 0, &res);
> if (ret)
> - return;
> + return -ENODEV;
>
> qe_ic = kzalloc(sizeof(*qe_ic), GFP_KERNEL);
> if (qe_ic == NULL)
> - return;
> + return -ENOMEM;
>
> qe_ic->irqhost = irq_domain_add_linear(node, NR_QE_IC_INTS,
> &qe_ic_host_ops, qe_ic);
> if (qe_ic->irqhost == NULL) {
> kfree(qe_ic);
> - return;
> + return -ENODEV;
> }
>
> qe_ic->regs = ioremap(res.start, resource_size(&res));
> @@ -437,7 +439,7 @@ static void __init qe_ic_init(struct device_node *node)
> if (!qe_ic->virq_low) {
> printk(KERN_ERR "Failed to map QE_IC low IRQ\n");
> kfree(qe_ic);
> - return;
> + return -ENODEV;
> }
> if (qe_ic->virq_high != qe_ic->virq_low) {
> low_handler = qe_ic_cascade_low;
> @@ -456,20 +458,26 @@ static void __init qe_ic_init(struct device_node *node)
> irq_set_handler_data(qe_ic->virq_high, qe_ic);
> irq_set_chained_handler(qe_ic->virq_high, high_handler);
> }
> + return 0;
> }
> +static const struct of_device_id qe_ic_ids[] = {
> + { .compatible = "fsl,qe-ic"},
> + { .compatible = "qeic"},
From the original code, this should be type = "qeic". It is not
defined in current binding but probably needed for backward
compatibility.
It would be great if you can also deal with the comments from Dan too. Thanks.
> + {},
> +};
>
> -static int __init qe_ic_of_init(void)
> +static struct platform_driver qe_ic_driver =
> {
> - struct device_node *np;
> + .driver = {
> + .name = "qe-ic",
> + .of_match_table = qe_ic_ids,
> + },
> + .probe = qe_ic_init,
> +};
>
> - np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
> - if (!np) {
> - np = of_find_node_by_type(NULL, "qeic");
> - if (!np)
> - return -ENODEV;
> - }
> - qe_ic_init(np);
> - of_node_put(np);
> +static int __init qe_ic_of_init(void)
> +{
> + platform_driver_register(&qe_ic_driver);
> return 0;
> }
> subsys_initcall(qe_ic_of_init);
> --
> 2.31.1
>
^ permalink raw reply
* Re: [PATCH v3 2/5] dt-bindings: nintendo-otp: Document the Wii and Wii U OTP support
From: Rob Herring @ 2021-07-14 22:51 UTC (permalink / raw)
To: Emmanuel Gil Peyrot
Cc: devicetree, linux-kernel, Srinivas Kandagatla, Ash Logan,
Paul Mackerras, linuxppc-dev, Jonathan Neuschäfer
In-Reply-To: <20210701225743.14631-3-linkmauve@linkmauve.fr>
On Fri, Jul 02, 2021 at 12:57:40AM +0200, Emmanuel Gil Peyrot wrote:
> Both of these consoles use the exact same two registers, even at the
> same address, but the Wii U has eight banks of 128 bytes memory while
> the Wii only has one, hence the two compatible strings.
>
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
> .../bindings/nvmem/nintendo-otp.yaml | 44 +++++++++++++++++++
> 1 file changed, 44 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
>
> diff --git a/Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml b/Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
> new file mode 100644
> index 000000000000..c39bd64b03b9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/nintendo-otp.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: GPL-2.0
GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/nintendo-otp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nintendo Wii and Wii U OTP Device Tree Bindings
> +
> +description: |
> + This binding represents the OTP memory as found on a Nintendo Wii or Wii U,
> + which contains common and per-console keys, signatures and related data
> + required to access peripherals.
> +
> + See https://wiiubrew.org/wiki/Hardware/OTP
> +
> +maintainers:
> + - Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> +
> +allOf:
> + - $ref: "nvmem.yaml#"
> +
> +properties:
> + compatible:
> + enum:
> + - nintendo,hollywood-otp
> + - nintendo,latte-otp
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + otp@d8001ec {
> + compatible = "nintendo,latte-otp";
> + reg = <0x0d8001ec 0x8>;
> + };
> +
> +...
> --
> 2.32.0
>
>
^ permalink raw reply
* RE: [EXT] [ppc][dlpar] WARNING: CPU: 2 PID: 1131623 at drivers/scsi/qla2xxx/qla_init.c:498 qla2x00_async_adisc_sp_done+0x294/0x2b0
From: Quinn Tran @ 2021-07-15 0:15 UTC (permalink / raw)
To: Abdul Haleem, linux-scsi
Cc: loberman@redhat.com, Martin K. Petersen,
himanshu.madhani@oracle.com, linux-kernel, Nilesh Javali,
Brian King, linuxppc-dev
In-Reply-To: <55e4b774-5fa5-b0d7-ef57-0a62fa05f090@linux.vnet.ibm.com>
Abdul,
For this test case, the warning can be treated as cometic. During this port disable / driver unload path, driver flush all command but fail to set the proper error code in this path. A recent patch does a warning if the error code is not part of the error code list. Regardless, the same outcome is achieved.
Will follow with a patch to suppress the warning.
commit 724361921f65a40ae5b80641dc1e92c0ff314d89
Author: Bart Van Assche <bvanassche@acm.org>
Date: Thu Aug 8 20:02:14 2019 -0700
scsi: qla2xxx: Report invalid mailbox status codes
It is easy to mix up the QLA_* and the MBS_* status codes. Complain loudly
if that happens.
Regards,
Quinn Tran
-----Original Message-----
From: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Sent: Monday, July 12, 2021 11:23 AM
To: linux-scsi <linux-scsi@vger.kernel.org>
Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>; linux-kernel <linux-kernel@vger.kernel.org>; loberman@redhat.com; Quinn Tran <qutran@marvell.com>; Nilesh Javali <njavali@marvell.com>; Martin K. Petersen <martin.petersen@oracle.com>; himanshu.madhani@oracle.com; Brian King <brking@linux.vnet.ibm.com>; Michael Ellerman <mpe@ellerman.id.au>
Subject: [EXT] [ppc][dlpar] WARNING: CPU: 2 PID: 1131623 at drivers/scsi/qla2xxx/qla_init.c:498 qla2x00_async_adisc_sp_done+0x294/0x2b0
External Email
----------------------------------------------------------------------
Greeting's
DLPAR hotunplug of FC adapter results in WARN_ONCE on my powerpc box
dmesg logs:
qla2xxx [0007:01:00.0]-2072:2: Async-login - 21:00:f4:e9:d4:54:a7:0f hdl=0, loopid=3 portid=0f1100 retries=27.
qla2xxx [0007:01:00.0]-b079:2: Removing driver qla2xxx [0007:01:00.0]-00af:2: Performing ISP error recovery - ha=0000000009e61ed1.
qla2xxx [0007:01:00.0]-2116:2: unregister localport=000000004d1de818 qla2xxx [0007:01:00.0]-210f:2: localport delete of 000000004d1de818 completed.
scsi 2:0:0:1: alua: Detached
scsi 2:0:1:0: alua: Detached
qla2xxx [0007:01:00.1]-5013:1: RSCN database changed -- 000f 0100 0000.
scsi 2:0:0:2: alua: Detached
scsi 2:0:1:1: alua: Detached
scsi 2:0:0:3: alua: Detached
scsi 2:0:0:0: alua: Detached
scsi 2:0:1:2: alua: Detached
scsi 2:0:0:4: alua: Detached
scsi 2:0:1:3: alua: Detached
scsi 2:0:0:5: alua: Detached
scsi 2:0:1:4: alua: Detached
scsi 2:0:1:5: alua: Detached
pci 0007:01:00.0: Removing from iommu group 0 qla2xxx [0007:01:00.1]-b079:1: Removing driver qla2xxx [0007:01:00.1]-00af:1: Performing ISP error recovery - ha=00000000c0615263.
------------[ cut here ]------------
mbs: 0x0
WARNING: CPU: 2 PID: 1131623 at drivers/scsi/qla2xxx/qla_init.c:498 qla2x00_async_adisc_sp_done+0x294/0x2b0 [qla2xxx] Modules linked in: isofs cdrom dm_round_robin dm_queue_length vfat fat btrfs blake2b_generic zstd_compress loop xfs raid0 rpadlpar_io rpaphp nfnetlink tcp_diag udp_diag inet_diag unix_diag af_packet_diag netlink_diag bonding rfkill sunrpc raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c pseries_rng xts vmx_crypto gf128mul sch_fq_codel binfmt_misc ip_tables ext4 mbcache jbd2 dm_service_time sd_mod sg qla2xxx ibmvfc ibmveth nvme_fc nvme_fabrics nvme_core t10_pi scsi_transport_fc dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse
CPU: 2 PID: 1131623 Comm: drmgr Not tainted 5.13.0-rc1-autotest #1
NIP: c00800000790857c LR: c008000007908578 CTR: 00000000006cdb9c
REGS: c00000001cdc3380 TRAP: 0700 Not tainted (5.13.0-rc1-autotest)
MSR: 800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 48002224 XER: 00000009
CFAR: c00000000013fd00 IRQMASK: 1
GPR00: c008000007908578 c00000001cdc3620 c0080000079d3000 0000000000000008
GPR04: 00000000ffff7fff c00000001cdc32e0 0000000000000027 c0000009ff5d7e08
GPR08: 0000000000000023 0000000000000001 0000000000000027 c0000000017e72b8
GPR12: 0000000000002000 c0000009ffffd680 0000000000000000 c000000aaa9d6000
GPR16: c00000002bd19000 0000000000000000 c000000022a06600 c000000aaa9c8890
GPR20: c00000003e4b5040 000000000000ffff 0004400000000000 000000000000f003
GPR24: 0000000000000001 0000000000000000 0000000000000000 ffffffffcccccccd
GPR28: c000000aaa9c8890 c0080000079ab678 c00000140a104800 c00000002bd19000 NIP [c00800000790857c] qla2x00_async_adisc_sp_done+0x294/0x2b0 [qla2xxx] LR [c008000007908578] qla2x00_async_adisc_sp_done+0x290/0x2b0 [qla2xxx] Call Trace:
[c00000001cdc3620] [c008000007908578] qla2x00_async_adisc_sp_done+0x290/0x2b0 [qla2xxx] (unreliable) [c00000001cdc3710] [c0080000078f3080] __qla2x00_abort_all_cmds+0x1b8/0x580 [qla2xxx] [c00000001cdc3840] [c0080000078f589c] qla2x00_abort_all_cmds+0x34/0xd0 [qla2xxx] [c00000001cdc3880] [c0080000079153d8] qla2x00_abort_isp_cleanup+0x3f0/0x570 [qla2xxx] [c00000001cdc3920] [c0080000078fb7e8] qla2x00_remove_one+0x3d0/0x480 [qla2xxx] [c00000001cdc39b0] [c00000000071c274] pci_device_remove+0x64/0x120 [c00000001cdc39f0] [c0000000007fb818] device_release_driver_internal+0x168/0x2a0
[c00000001cdc3a30] [c00000000070e304] pci_stop_bus_device+0xb4/0x100 [c00000001cdc3a70] [c00000000070e4f0] pci_stop_and_remove_bus_device+0x20/0x40
[c00000001cdc3aa0] [c000000000073940] pci_hp_remove_devices+0x90/0x130 [c00000001cdc3b30] [c0080000070704d0] disable_slot+0x38/0x90 [rpaphp] [c00000001cdc3b60] [c00000000073eb4c] power_write_file+0xcc/0x180 [c00000001cdc3be0] [c0000000007354bc] pci_slot_attr_store+0x3c/0x60 [c00000001cdc3c00] [c00000000055f820] sysfs_kf_write+0x60/0x80 [c00000001cdc3c20] [c00000000055df10] kernfs_fop_write_iter+0x1a0/0x290 [c00000001cdc3c70] [c000000000447c4c] new_sync_write+0x14c/0x1d0 [c00000001cdc3d10] [c00000000044b134] vfs_write+0x224/0x330 [c00000001cdc3d60] [c00000000044b3f4] ksys_write+0x74/0x130 [c00000001cdc3db0] [c00000000002df70] system_call_exception+0x150/0x2d0 [c00000001cdc3e10] [c00000000000d45c] system_call_common+0xec/0x278
--- interrupt: c00 at 0x20000026bd74
NIP: 000020000026bd74 LR: 00002000001e34c4 CTR: 0000000000000000
REGS: c00000001cdc3e80 TRAP: 0c00 Not tainted (5.13.0-rc1-autotest)
MSR: 800000000280f033 <SF,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE> CR: 24004242 XER: 00000000
IRQMASK: 0
GPR00: 0000000000000004 00007ffff738f460 0000200000367100 0000000000000006
GPR04: 00000001588d4c90 0000000000000001 fffffffffbad2c84 00000001588d02a0
GPR08: 0000000000000001 0000000000000000 0000000000000000 0000000000000000
GPR12: 0000000000000000 000020000005b520 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR24: 00000001588e3104 0000000000000000 0000000000000001 00000001588d4c90
GPR28: 0000000000000001 00000001588d02a0 00000001588d4c90 0000000000000001 NIP [000020000026bd74] 0x20000026bd74 LR [00002000001e34c4] 0x2000001e34c4
--- interrupt: c00
Instruction dump:
4bffff20 3d420000 e94a8cd0 892a0003 2f890000 409efeb0 3c620000 e8638e70
39200001 992a0003 4809a80d e8410018 <0fe00000> a08100ac 4bfffe8c 4809b771 ---[ end trace 31eef07e575ca89a ]--- qla2xxx [0007:01:00.1]-f085:1: Schedule logo failed sess 000000003350b45f rc 258 qla2xxx [0007:01:00.1]-2116:1: unregister localport=00000000c3a73a3b qla2xxx [0007:01:00.1]-210f:1: localport delete of 00000000c3a73a3b completed.
scsi 1:0:0:0: alua: Detached
scsi 1:0:0:1: alua: Detached
pci 0007:01:00.1: Removing from iommu group 0 scsi 1:0:1:0: alua: Detached scsi 1:0:1:1: alua: Detached scsi 1:0:0:2: alua: Detached scsi 1:0:0:3: alua: Detached scsi 1:0:1:2: alua: Detached scsi 1:0:1:3: alua: Detached scsi 1:0:0:4: alua: Detached scsi 1:0:0:5: alua: Detached scsi 1:0:1:4: alua: Detached scsi 1:0:1:5: alua: Detached pci_bus 0007:01: busn_res: [bus 01-ff] is released
rpadlpar_io: slot PHB 18 removed
PCI host bridge /pci@800000020000012 ranges:
MEM 0x0000040080000000..0x00000400feffffff -> 0x0000000080000000
MEM 0x0000044000000000..0x0000047fffffffff -> 0x0006220000000000 PCI host bridge to bus 0008:01 pci_bus 0008:01: root bus resource [mem 0x40080000000-0x400feffffff] (bus address [0x80000000-0xfeffffff]) pci_bus 0008:01: root bus resource [mem 0x44000000000-0x47fffffffff 64bit] (bus address [0x6220000000000-0x6223fffffffff])
pci_bus 0008:01: root bus resource [bus 01-ff] pci 0008:01:00.1: No hypervisor support for SR-IOV on this device, IOV BARs disabled.
pci 0008:01:00.0: No hypervisor support for SR-IOV on this device, IOV BARs disabled.
pci 0008:01:00.1: Adding to iommu group 0 qla2xxx 0008:01:00.1: enabling device (0140 -> 0142) qla2xxx [0000:00:00.0]-011c: : MSI-X vector count: 32.
qla2xxx [0000:00:00.0]-001d: : Found an ISP2261 irq 19 iobase 0x00000000e9bdc504.
qla2xxx 0008:01:00.1: ibm,query-pe-dma-windows(53) 10100 8000000 20000012 returned 0 qla2xxx 0008:01:00.1: ibm,create-pe-dma-window(54) 10100 8000000 20000012 10 25 returned 0 (liobn = 0x70000012 starting addr = 8000000 0) qla2xxx [0008:01:00.1]-0075:1: ZIO mode 6 enabled; timer delay (200 us).
qla2xxx [0008:01:00.1]-ffff:1: FC4 priority set to NVMe qla2xxx [0008:01:00.1]-d302:1: qla2x00_get_fw_version: FC-NVMe is Enabled (0x7858) qla2xxx [0008:01:00.1]-11a3:1: SCM in FW: Not Supported qla2xxx [0008:01:00.1]-500a:1: LOOP UP detected (16 Gbps).
scsi host1: qla2xxx
qla2xxx [0008:01:00.1]-00fb:1: QLogic QLE2692 - PCIe3 2-Port 16Gb Fibre Channel Adapter, FC PF.
qla2xxx [0008:01:00.1]-00fc:1: ISP2261: PCIe (8.0GT/s x8) @ 0008:01:00.1 hdma- host#=1 fw=8.08.06 (d0d5).
Machine: PowerPC LPAR
Adapter: FC adapter
Linux Repo: https://urldefense.proofpoint.com/v2/url?u=https-3A__git.kernel.org_pub_scm_linux_kernel_git_mkp_scsi.git&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=jE1dBr2AFjWUy-jpipHnTT4m17st97cMjIiDw5OmfEI&m=YpU52P6hWZZBNdC2bTgo9xnBNjzaVDNC8TsW4Ilsnes&s=JIMc-eX1l1GtjajKRQfsMqtrtdqRBN-cv8n91d9fy6w&e= (for-next)
test: DLPAR remove of FC adapter
@Quinn, Could you please have a look if it is because of recent code changes
--
Regard's
Abdul Haleem
IBM Linux Technology Cente
^ permalink raw reply
* Re: [PATCH v3 1/1] powerpc/pseries: Interface to represent PAPR firmware attributes
From: Pratik Sampat @ 2021-07-15 6:07 UTC (permalink / raw)
To: Fabiano Rosas, mpe, benh, paulus, linuxppc-dev, kvm-ppc,
linux-kernel, pratik.r.sampat
In-Reply-To: <87lf6bo7v0.fsf@linux.ibm.com>
Hello,
On 12/07/21 9:13 pm, Fabiano Rosas wrote:
> "Pratik R. Sampat" <psampat@linux.ibm.com> writes:
>
> Hi, have you seen Documentation/core-api/kobject.rst, particularly the
> part that says:
>
> "When you see a sysfs directory full of other directories, generally each
> of those directories corresponds to a kobject in the same kset."
>
> Taking a look at samples/kobject/kset-example.c, it seems to provide an
> overall structure that is closer to what other modules do when creating
> sysfs entries. It uses less dynamic allocations and deals a bit better
> with cleaning up the state afterwards.
>
Thank you for pointing me towards this example, the kset approach is
interesting and the example indeed does handle cleanups better.
Currently, we use "machine_device_initcall()" to register this
functionality, do you suggest I convert this into a tristate module
instead where I can include a "module_exit" for cleanups?
>> Adds a generic interface to represent the energy and frequency related
>> PAPR attributes on the system using the new H_CALL
>> "H_GET_ENERGY_SCALE_INFO".
>>
>> H_GET_EM_PARMS H_CALL was previously responsible for exporting this
>> information in the lparcfg, however the H_GET_EM_PARMS H_CALL
>> will be deprecated P10 onwards.
>>
>> The H_GET_ENERGY_SCALE_INFO H_CALL is of the following call format:
>> hcall(
>> uint64 H_GET_ENERGY_SCALE_INFO, // Get energy scale info
>> uint64 flags, // Per the flag request
>> uint64 firstAttributeId,// The attribute id
>> uint64 bufferAddress, // Guest physical address of the output buffer
>> uint64 bufferSize // The size in bytes of the output buffer
>> );
>>
>> This H_CALL can query either all the attributes at once with
>> firstAttributeId = 0, flags = 0 as well as query only one attribute
>> at a time with firstAttributeId = id, flags = 1.
>>
>> The output buffer consists of the following
>> 1. number of attributes - 8 bytes
>> 2. array offset to the data location - 8 bytes
>> 3. version info - 1 byte
>> 4. A data array of size num attributes, which contains the following:
>> a. attribute ID - 8 bytes
>> b. attribute value in number - 8 bytes
>> c. attribute name in string - 64 bytes
>> d. attribute value in string - 64 bytes
>>
>> The new H_CALL exports information in direct string value format, hence
>> a new interface has been introduced in
>> /sys/firmware/papr/energy_scale_info to export this information to
>> userspace in an extensible pass-through format.
>>
>> The H_CALL returns the name, numeric value and string value (if exists)
>>
>> The format of exposing the sysfs information is as follows:
>> /sys/firmware/papr/energy_scale_info/
>> |-- <id>/
>> |-- desc
>> |-- value
>> |-- value_desc (if exists)
>> |-- <id>/
>> |-- desc
>> |-- value
>> |-- value_desc (if exists)
>> ...
>>
>> The energy information that is exported is useful for userspace tools
>> such as powerpc-utils. Currently these tools infer the
>> "power_mode_data" value in the lparcfg, which in turn is obtained from
>> the to be deprecated H_GET_EM_PARMS H_CALL.
>> On future platforms, such userspace utilities will have to look at the
>> data returned from the new H_CALL being populated in this new sysfs
>> interface and report this information directly without the need of
>> interpretation.
>>
>> Signed-off-by: Pratik R. Sampat <psampat@linux.ibm.com>
>> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
>> ---
>> .../sysfs-firmware-papr-energy-scale-info | 26 ++
>> arch/powerpc/include/asm/hvcall.h | 24 +-
>> arch/powerpc/kvm/trace_hv.h | 1 +
>> arch/powerpc/platforms/pseries/Makefile | 3 +-
>> .../pseries/papr_platform_attributes.c | 320 ++++++++++++++++++
>> 5 files changed, 372 insertions(+), 2 deletions(-)
>> create mode 100644 Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
>> create mode 100644 arch/powerpc/platforms/pseries/papr_platform_attributes.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info b/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
>> new file mode 100644
>> index 000000000000..fd82f2bfafe5
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-firmware-papr-energy-scale-info
>> @@ -0,0 +1,26 @@
>> +What: /sys/firmware/papr/energy_scale_info
>> +Date: June 2021
>> +Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
>> +Description: Directory hosting a set of platform attributes like
>> + energy/frequency on Linux running as a PAPR guest.
>> +
>> + Each file in a directory contains a platform
>> + attribute hierarchy pertaining to performance/
>> + energy-savings mode and processor frequency.
>> +
>> +What: /sys/firmware/papr/energy_scale_info/<id>
>> + /sys/firmware/papr/energy_scale_info/<id>/desc
>> + /sys/firmware/papr/energy_scale_info/<id>/value
>> + /sys/firmware/papr/energy_scale_info/<id>/value_desc
>> +Date: June 2021
>> +Contact: Linux for PowerPC mailing list <linuxppc-dev@ozlabs.org>
>> +Description: Energy, frequency attributes directory for POWERVM servers
>> +
>> + This directory provides energy, erequency, folding information. It
> s/erequency/frequency/
ack. Thanks for pointing out the typo
>
>> + contains below sysfs attributes:
>> +
>> + - desc: String description of the attribute <id>
>> +
>> + - value: Numeric value of attribute <id>
>> +
>> + - value_desc: String value of attribute <id>
>> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
>> index e3b29eda8074..c91714ea6719 100644
>> --- a/arch/powerpc/include/asm/hvcall.h
>> +++ b/arch/powerpc/include/asm/hvcall.h
>> @@ -316,7 +316,8 @@
>> #define H_SCM_PERFORMANCE_STATS 0x418
>> #define H_RPT_INVALIDATE 0x448
>> #define H_SCM_FLUSH 0x44C
>> -#define MAX_HCALL_OPCODE H_SCM_FLUSH
>> +#define H_GET_ENERGY_SCALE_INFO 0x450
>> +#define MAX_HCALL_OPCODE H_GET_ENERGY_SCALE_INFO
>>
>> /* Scope args for H_SCM_UNBIND_ALL */
>> #define H_UNBIND_SCOPE_ALL (0x1)
>> @@ -631,6 +632,27 @@ struct hv_gpci_request_buffer {
>> uint8_t bytes[HGPCI_MAX_DATA_BYTES];
>> } __packed;
>>
>> +#define ESI_VERSION 0x1
>> +#define MAX_ESI_ATTRS 10
>> +#define MAX_BUF_SZ (sizeof(struct h_energy_scale_info_hdr) + \
>> + (sizeof(struct energy_scale_attribute) * MAX_ESI_ATTRS))
>> +
>> +struct energy_scale_attribute {
>> + __be64 id;
>> + __be64 value;
>> + unsigned char desc[64];
>> + unsigned char value_desc[64];
>> +} __packed;
>> +
>> +struct h_energy_scale_info_hdr {
>> + __be64 num_attrs;
>> + __be64 array_offset;
>> + __u8 data_header_version;
>> +} __packed;
>> +
>> +/* /sys/firmware/papr */
>> +extern struct kobject *papr_kobj;
>> +
>> #endif /* __ASSEMBLY__ */
>> #endif /* __KERNEL__ */
>> #endif /* _ASM_POWERPC_HVCALL_H */
>> diff --git a/arch/powerpc/kvm/trace_hv.h b/arch/powerpc/kvm/trace_hv.h
>> index 830a126e095d..38cd0ed0a617 100644
>> --- a/arch/powerpc/kvm/trace_hv.h
>> +++ b/arch/powerpc/kvm/trace_hv.h
>> @@ -115,6 +115,7 @@
>> {H_VASI_STATE, "H_VASI_STATE"}, \
>> {H_ENABLE_CRQ, "H_ENABLE_CRQ"}, \
>> {H_GET_EM_PARMS, "H_GET_EM_PARMS"}, \
>> + {H_GET_ENERGY_SCALE_INFO, "H_GET_ENERGY_SCALE_INFO"}, \
>> {H_SET_MPP, "H_SET_MPP"}, \
>> {H_GET_MPP, "H_GET_MPP"}, \
>> {H_HOME_NODE_ASSOCIATIVITY, "H_HOME_NODE_ASSOCIATIVITY"}, \
>> diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
>> index c8a2b0b05ac0..d14fca89ac25 100644
>> --- a/arch/powerpc/platforms/pseries/Makefile
>> +++ b/arch/powerpc/platforms/pseries/Makefile
>> @@ -6,7 +6,8 @@ obj-y := lpar.o hvCall.o nvram.o reconfig.o \
>> of_helpers.o \
>> setup.o iommu.o event_sources.o ras.o \
>> firmware.o power.o dlpar.o mobility.o rng.o \
>> - pci.o pci_dlpar.o eeh_pseries.o msi.o
>> + pci.o pci_dlpar.o eeh_pseries.o msi.o \
>> + papr_platform_attributes.o
>> obj-$(CONFIG_SMP) += smp.o
>> obj-$(CONFIG_SCANLOG) += scanlog.o
>> obj-$(CONFIG_KEXEC_CORE) += kexec.o
>> diff --git a/arch/powerpc/platforms/pseries/papr_platform_attributes.c b/arch/powerpc/platforms/pseries/papr_platform_attributes.c
>> new file mode 100644
>> index 000000000000..43c620132b4d
>> --- /dev/null
>> +++ b/arch/powerpc/platforms/pseries/papr_platform_attributes.c
>> @@ -0,0 +1,320 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Platform energy and frequency attributes driver
>> + *
>> + * This driver creates a sys file at /sys/firmware/papr/ which encapsulates a
>> + * directory structure containing files in keyword - value pairs that specify
>> + * energy and frequency configuration of the system.
>> + *
>> + * The format of exposing the sysfs information is as follows:
>> + * /sys/firmware/papr/energy_scale_info/
>> + * |-- <id>/
>> + * |-- desc
>> + * |-- value
>> + * |-- value_desc (if exists)
>> + * |-- <id>/
>> + * |-- desc
>> + * |-- value
>> + * |-- value_desc (if exists)
>> + *
>> + * Copyright 2021 IBM Corp.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/types.h>
>> +#include <linux/errno.h>
>> +#include <linux/init.h>
>> +#include <linux/seq_file.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/hugetlb.h>
>> +#include <asm/lppaca.h>
>> +#include <asm/hvcall.h>
>> +#include <asm/firmware.h>
>> +#include <asm/time.h>
>> +#include <asm/prom.h>
>> +#include <asm/vdso_datapage.h>
>> +#include <asm/vio.h>
>> +#include <asm/mmu.h>
>> +#include <asm/machdep.h>
>> +#include <asm/drmem.h>
>> +
>> +#include "pseries.h"
>> +
>> +#define MAX_ATTRS 3
>> +#define MAX_NAME_LEN 16
>> +
>> +/*
>> + * Flag attributes to fetch either all or one attribute from the HCALL
>> + * flag = BE(0) => fetch all attributes with firstAttributeId = 0
>> + * flag = BE(1) => fetch a single attribute with firstAttributeId = id
>> + */
>> +#define ESI_FLAGS_ALL 0
>> +#define ESI_FLAGS_SINGLE PPC_BIT(0)
>> +
>> +struct papr_attr {
>> + u64 id;
>> + struct kobj_attribute kobj_attr;
>> +};
>> +struct papr_group {
>> + char name[MAX_NAME_LEN];
> This is unused.
ack. We used it in one of the previous iterations and I missed removing it.
Thanks.
>
>> + struct attribute_group pg;
>> + struct papr_attr *pgattrs;
> This is always MAX_ATTRS long, we could define it statically and avoid a
> kmalloc later.
Sure
>> +} *pgs;
>> +
>> +/* /sys/firmware/papr */
>> +struct kobject *papr_kobj;
>> +/* /sys/firmware/papr/energy_scale_info */
>> +struct kobject *esi_kobj;
>> +
>> +struct h_energy_scale_info_hdr *esi_hdr;
>> +struct energy_scale_attribute *esi_attrs;
>> +
>> +/*
>> + * Extract and export the description of the energy scale attribute
>> + *
>> + * As We do not expect the name to change, hence use the old description and
>> + * save a call to the H_GET_ENERGY_SCALE_INFO HCALL
>> + */
>> +static ssize_t papr_show_desc(struct kobject *kobj,
>> + struct kobj_attribute *kobj_attr,
>> + char *buf)
>> +{
>> + struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
>> + kobj_attr);
>> + int idx, ret = 0;
>> +
>> + for (idx = 0; idx < be64_to_cpu(esi_hdr->num_attrs); idx++) {
>> + if (pattr->id == be64_to_cpu(esi_attrs[idx].id)) {
>> + ret = sprintf(buf, "%s\n", esi_attrs[idx].desc);
> We know the size of desc, so could use snprintf here.
ack
>> + if (ret < 0)
>> + ret = -EIO;
>> + break;
>> + }
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> + * Extract and export the numeric value of the energy scale attributes
>> + */
>> +static ssize_t papr_show_value(struct kobject *kobj,
>> + struct kobj_attribute *kobj_attr,
>> + char *buf)
>> +{
>> + struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
>> + kobj_attr);
>> + char *t_buf;
>> + struct h_energy_scale_info_hdr *t_hdr;
>> + struct energy_scale_attribute *t_esi;
>> + int ret = 0;
>> +
>> + t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
>> + if (t_buf == NULL)
>> + return -ENOMEM;
>> +
>> + ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
>> + pattr->id, virt_to_phys(t_buf),
>> + MAX_BUF_SZ);
>> +
>> + if (ret != H_SUCCESS) {
>> + pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>> + goto out;
>> + }
>> +
>> + t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
>> + t_esi = (struct energy_scale_attribute *)
>> + (t_buf + be64_to_cpu(t_hdr->array_offset));
>> +
>> + ret = sprintf(buf, "%llu\n", be64_to_cpu(t_esi->value));
> snprintf
ack
>> + if (ret < 0)
>> + ret = -EIO;
>> +out:
>> + kfree(t_buf);
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> + * Extract and export the value description in string format of the energy
>> + * scale attributes
>> + */
>> +static ssize_t papr_show_value_desc(struct kobject *kobj,
>> + struct kobj_attribute *kobj_attr,
>> + char *buf)
>> +{
>> + struct papr_attr *pattr = container_of(kobj_attr, struct papr_attr,
>> + kobj_attr);
>> + char *t_buf;
>> + struct h_energy_scale_info_hdr *t_hdr;
>> + struct energy_scale_attribute *t_esi;
>> + int ret = 0;
>> +
>> + t_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
>> + if (t_buf == NULL)
>> + return -ENOMEM;
>> +
>> + ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_SINGLE,
>> + pattr->id, virt_to_phys(t_buf),
>> + MAX_BUF_SZ);
>> +
>> + if (ret != H_SUCCESS) {
>> + pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>> + goto out;
>> + }
>> +
>> + t_hdr = (struct h_energy_scale_info_hdr *) t_buf;
>> + t_esi = (struct energy_scale_attribute *)
>> + (t_buf + be64_to_cpu(t_hdr->array_offset));
>> +
>> + ret = sprintf(buf, "%s\n", t_esi->value_desc);
> snprintf
>
ack
>> + if (ret < 0)
>> + ret = -EIO;
>> +out:
>> + kfree(t_buf);
>> +
>> + return ret;
>> +}
>> +
>> +static struct papr_ops_info {
>> + const char *attr_name;
>> + ssize_t (*show)(struct kobject *kobj, struct kobj_attribute *kobj_attr,
>> + char *buf);
>> +} ops_info[MAX_ATTRS] = {
>> + { "desc", papr_show_desc },
>> + { "value", papr_show_value },
>> + { "value_desc", papr_show_value_desc },
>> +};
>> +
>> +static void add_attr(u64 id, int index, struct papr_attr *attr)
>> +{
>> + attr->id = id;
>> + sysfs_attr_init(&attr->kobj_attr.attr);
>> + attr->kobj_attr.attr.name = ops_info[index].attr_name;
>> + attr->kobj_attr.attr.mode = 0444;
>> + attr->kobj_attr.show = ops_info[index].show;
>> +}
>> +
>> +static int add_attr_group(u64 id, int len, struct papr_group *pg,
>> + bool show_val_desc)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < len; i++) {
>> + if (!strcmp(ops_info[i].attr_name, "value_desc") &&
>> + !show_val_desc) {
>> + continue;
>> + }
>> + add_attr(id, i, &pg->pgattrs[i]);
>> + pg->pg.attrs[i] = &pg->pgattrs[i].kobj_attr.attr;
>> + }
>> +
>> + return sysfs_create_group(esi_kobj, &pg->pg);
>> +}
>> +
>> +static int __init papr_init(void)
>> +{
>> + uint64_t num_attrs;
>> + int ret, idx, i;
>> + char *esi_buf;
>> +
>> + if (!firmware_has_feature(FW_FEATURE_LPAR))
>> + return -ENXIO;
>> +
>> + esi_buf = kmalloc(MAX_BUF_SZ, GFP_KERNEL);
> This memory is never released. I don't think it's worth it just to save
> a hcall.
Sure, I can convert the struct to local scope in this function and make
a H_CALL for "desc"
>> + if (esi_buf == NULL)
>> + return -ENOMEM;
>> + /*
>> + * hcall(
>> + * uint64 H_GET_ENERGY_SCALE_INFO, // Get energy scale info
>> + * uint64 flags, // Per the flag request
>> + * uint64 firstAttributeId, // The attribute id
>> + * uint64 bufferAddress, // Guest physical address of the output buffer
>> + * uint64 bufferSize); // The size in bytes of the output buffer
>> + */
>> + ret = plpar_hcall_norets(H_GET_ENERGY_SCALE_INFO, ESI_FLAGS_ALL, 0,
>> + virt_to_phys(esi_buf), MAX_BUF_SZ);
>> + esi_hdr = (struct h_energy_scale_info_hdr *) esi_buf;
>> + if (ret != H_SUCCESS || esi_hdr->data_header_version != ESI_VERSION) {
> I really dislike this. If you want to bail due to version change, then
> at least include in the ABI document that we might not give the
> userspace any data at all.
My only concern for having a version check is that, the attribute list
can change as well as the attributes itself may change.
If that is the case, then in a newer version if we do not bail out we
may parse data into our structs incorrectly.
My argument only hinges on that we should likely give no data at all
instead of junk or incorrect data.
Maybe I could make this check after the return check and give out a
version mismatch message like the following?
pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO VER MISMATCH - EXP: 0x%x, REC: 0x%x",
ESI_VERSION, esi_hdr->data_header_version);
>> + pr_warn("hcall failed: H_GET_ENERGY_SCALE_INFO");
>> + goto out;
>> + }
>> +
>> + num_attrs = be64_to_cpu(esi_hdr->num_attrs);
>> + /*
>> + * Typecast the energy buffer to the attribute structure at the offset
>> + * specified in the buffer
>> + */
> I think the code is now simple enough that this comment could be
> removed.
ack
>> + esi_attrs = (struct energy_scale_attribute *)
>> + (esi_buf + be64_to_cpu(esi_hdr->array_offset));
>> +
>> + pgs = kcalloc(num_attrs, sizeof(*pgs), GFP_KERNEL);
> This is never freed.
>
>> + if (!pgs)
>> + goto out_pgs;
>> +
>> + papr_kobj = kobject_create_and_add("papr", firmware_kobj);
>> + if (!papr_kobj) {
>> + pr_warn("kobject_create_and_add papr failed\n");
>> + goto out_kobj;
>> + }
>> +
>> + esi_kobj = kobject_create_and_add("energy_scale_info", papr_kobj);
>> + if (!esi_kobj) {
>> + pr_warn("kobject_create_and_add energy_scale_info failed\n");
>> + goto out_ekobj;
>> + }
>> +
>> + for (idx = 0; idx < num_attrs; idx++) {
>> + char buf[4];
>> + bool show_val_desc = true;
>> +
>> + pgs[idx].pgattrs = kcalloc(MAX_ATTRS,
>> + sizeof(*pgs[idx].pgattrs),
>> + GFP_KERNEL);
>> + if (!pgs[idx].pgattrs)
>> + goto out_kobj;
>> +
>> + pgs[idx].pg.attrs = kcalloc(MAX_ATTRS + 1,
>> + sizeof(*pgs[idx].pg.attrs),
>> + GFP_KERNEL);
> I think the kobject code expects this to be statically allocated, so
> you'd need to override the release function in some way to be able to
> free this.
Right this and pgs both are never free'd because my understanding was
that as this functionality is invoked from machine_init, I'd expect it
to stay until shutdown.
However, if you believe that a module approach is cleaner, I can change
my implementation to accommodate for that and also include a
module_exit for cleanup of the above allocations
>
>> + if (!pgs[idx].pg.attrs) {
>> + kfree(pgs[idx].pgattrs);
>> + goto out_kobj;
>> + }
>> +
>> + sprintf(buf, "%lld", be64_to_cpu(esi_attrs[idx].id));
> Do you mean pgs[idx].name instead of buf? Otherwise you're passing this
> stack allocated 'buf' to another function.
>
Yes you're right I should have either passed the pg struct or I should
have used strcpy, here the stack allocated buffer is being taken out of
scope which is incorrect.
Thanks for pointing this out!
>> + pgs[idx].pg.name = buf;
>> +
>> + /* Do not add the value description if it does not exist */
>> + if (strlen(esi_attrs[idx].value_desc) == 0)
>> + show_val_desc = false;
>> +
>> + if (add_attr_group(be64_to_cpu(esi_attrs[idx].id),
>> + MAX_ATTRS, &pgs[idx], show_val_desc)) {
>> + pr_warn("Failed to create papr attribute group %s\n",
>> + pgs[idx].pg.name);
>> + goto out_pgattrs;
>> + }
>> + }
>> +
>> + return 0;
>> +
>> +out_pgattrs:
>> + for (i = 0; i < MAX_ATTRS; i++) {
>> + kfree(pgs[i].pgattrs);
>> + kfree(pgs[i].pg.attrs);
>> + }
>> +out_ekobj:
>> + kobject_put(esi_kobj);
>> +out_kobj:
>> + kobject_put(papr_kobj);
>> +out_pgs:
>> + kfree(pgs);
>> +out:
>> + kfree(esi_buf);
>> +
>> + return -ENOMEM;
>> +}
>> +
>> +machine_device_initcall(pseries, papr_init);
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV P9: Fix guest TM support
From: Michael Ellerman @ 2021-07-15 12:11 UTC (permalink / raw)
To: Nicholas Piggin, kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev
In-Reply-To: <20210712013650.376325-1-npiggin@gmail.com>
On Mon, 12 Jul 2021 11:36:50 +1000, Nicholas Piggin wrote:
> The conversion to C introduced several bugs in TM handling that can
> cause host crashes with TM bad thing interrupts. Mostly just simple
> typos or missed logic in the conversion that got through due to my
> not testing TM in the guest sufficiently.
>
> - Early TM emulation for the softpatch interrupt should be done if fake
> suspend mode is _not_ active.
>
> [...]
Applied to powerpc/fixes.
[1/1] KVM: PPC: Book3S HV P9: Fix guest TM support
https://git.kernel.org/powerpc/c/e44fbdb68049539de9923ce4bad2d277aef54892
cheers
^ permalink raw reply
* [RFC PATCH 0/6] KVM: PPC: Book3S HV P9: Reduce guest entry/exit
From: Nicholas Piggin @ 2021-07-15 13:14 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
This goes on top of the previous speedup series. The previous series is
mostly invovled with reducing the cost of SPR accesses. This one starts
to look beyond those, to atomics, barriers, and other logic that can be
reduced. After this series the P9 path uses very few things from the
vcore structure. This saves several hundred cycles for guest entry/exit
on a POWER9.
Thanks,
Nick
Nicholas Piggin (6):
KVM: PPC: Book3S HV P9: Add unlikely annotation for !mmu_ready
KVM: PPC: Book3S HV P9: Avoid cpu_in_guest atomics on entry and exit
KVM: PPC: Book3S HV P9: Remove most of the vcore logic
KVM: PPC: Book3S HV P9: Tidy kvmppc_create_dtl_entry
KVM: PPC: Book3S HV P9: Stop using vc->dpdes
KVM: PPC: Book3S HV P9: Remove subcore HMI handling
arch/powerpc/include/asm/kvm_book3s_64.h | 1 -
arch/powerpc/include/asm/kvm_host.h | 1 -
arch/powerpc/kvm/book3s_hv.c | 250 +++++++++++++----------
arch/powerpc/kvm/book3s_hv_builtin.c | 2 +
arch/powerpc/kvm/book3s_hv_hmi.c | 7 +-
arch/powerpc/kvm/book3s_hv_p9_entry.c | 35 +++-
arch/powerpc/kvm/book3s_hv_ras.c | 4 +
7 files changed, 185 insertions(+), 115 deletions(-)
--
2.23.0
^ permalink raw reply
* [RFC PATCH 0/6] KVM: PPC: Book3S HV P9: Reduce guest entry/exit
From: Nicholas Piggin @ 2021-07-15 13:15 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
This goes on top of the previous speedup series. The previous series is
mostly invovled with reducing the cost of SPR accesses. This one starts
to look beyond those, to atomics, barriers, and other logic that can be
reduced. After this series the P9 path uses very few things from the
vcore structure. This saves several hundred cycles for guest entry/exit
on a POWER9.
Thanks,
Nick
Nicholas Piggin (6):
KVM: PPC: Book3S HV P9: Add unlikely annotation for !mmu_ready
KVM: PPC: Book3S HV P9: Avoid cpu_in_guest atomics on entry and exit
KVM: PPC: Book3S HV P9: Remove most of the vcore logic
KVM: PPC: Book3S HV P9: Tidy kvmppc_create_dtl_entry
KVM: PPC: Book3S HV P9: Stop using vc->dpdes
KVM: PPC: Book3S HV P9: Remove subcore HMI handling
arch/powerpc/include/asm/kvm_book3s_64.h | 1 -
arch/powerpc/include/asm/kvm_host.h | 1 -
arch/powerpc/kvm/book3s_hv.c | 250 +++++++++++++----------
arch/powerpc/kvm/book3s_hv_builtin.c | 2 +
arch/powerpc/kvm/book3s_hv_hmi.c | 7 +-
arch/powerpc/kvm/book3s_hv_p9_entry.c | 35 +++-
arch/powerpc/kvm/book3s_hv_ras.c | 4 +
7 files changed, 185 insertions(+), 115 deletions(-)
--
2.23.0
^ permalink raw reply
* [RFC PATCH 1/6] KVM: PPC: Book3S HV P9: Add unlikely annotation for !mmu_ready
From: Nicholas Piggin @ 2021-07-15 13:15 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210715131518.146917-1-npiggin@gmail.com>
The mmu will almost always be ready.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 9d15bbafe333..27a7a856eed1 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4367,7 +4367,7 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
vc->runner = vcpu;
/* See if the MMU is ready to go */
- if (!kvm->arch.mmu_ready) {
+ if (unlikely(!kvm->arch.mmu_ready)) {
r = kvmhv_setup_mmu(vcpu);
if (r) {
run->exit_reason = KVM_EXIT_FAIL_ENTRY;
--
2.23.0
^ permalink raw reply related
* [RFC PATCH 2/6] KVM: PPC: Book3S HV P9: Avoid cpu_in_guest atomics on entry and exit
From: Nicholas Piggin @ 2021-07-15 13:15 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210715131518.146917-1-npiggin@gmail.com>
cpu_in_guest is set to determine if a CPU needs to be IPI'ed to exit
the guest and notice the need_tlb_flush bit.
This can be implemented as a global per-CPU pointer to the currently
running guest instead of per-guest cpumasks, saving 2 atomics per
entry/exit. P7/8 doesn't require cpu_in_guest, nor does a nested HV
(only the L0 does), so move it to the P9 HV path.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/kvm_book3s_64.h | 1 -
arch/powerpc/include/asm/kvm_host.h | 1 -
arch/powerpc/kvm/book3s_hv.c | 38 +++++++++++++-----------
3 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 20ca9b1a2d41..2b442e00fb5d 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -43,7 +43,6 @@ struct kvm_nested_guest {
struct mutex tlb_lock; /* serialize page faults and tlbies */
struct kvm_nested_guest *next;
cpumask_t need_tlb_flush;
- cpumask_t cpu_in_guest;
short prev_cpu[NR_CPUS];
u8 radix; /* is this nested guest radix */
};
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index cd7939eb47ca..45dc92812020 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -288,7 +288,6 @@ struct kvm_arch {
u32 online_vcores;
atomic_t hpte_mod_interest;
cpumask_t need_tlb_flush;
- cpumask_t cpu_in_guest;
u8 radix;
u8 fwnmi_enabled;
u8 secure_guest;
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 27a7a856eed1..d809566918de 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -2981,30 +2981,33 @@ static void kvmppc_release_hwthread(int cpu)
tpaca->kvm_hstate.kvm_split_mode = NULL;
}
+static DEFINE_PER_CPU(struct kvm *, cpu_in_guest);
+
static void radix_flush_cpu(struct kvm *kvm, int cpu, struct kvm_vcpu *vcpu)
{
struct kvm_nested_guest *nested = vcpu->arch.nested;
- cpumask_t *cpu_in_guest;
int i;
cpu = cpu_first_tlb_thread_sibling(cpu);
- if (nested) {
+ if (nested)
cpumask_set_cpu(cpu, &nested->need_tlb_flush);
- cpu_in_guest = &nested->cpu_in_guest;
- } else {
+ else
cpumask_set_cpu(cpu, &kvm->arch.need_tlb_flush);
- cpu_in_guest = &kvm->arch.cpu_in_guest;
- }
/*
- * Make sure setting of bit in need_tlb_flush precedes
- * testing of cpu_in_guest bits. The matching barrier on
- * the other side is the first smp_mb() in kvmppc_run_core().
+ * Make sure setting of bit in need_tlb_flush precedes testing of
+ * cpu_in_guest. The matching barrier on the other side is hwsync
+ * when switching to guest MMU mode, which happens between
+ * cpu_in_guest being set to the guest kvm, and need_tlb_flush bit
+ * being tested.
*/
smp_mb();
for (i = cpu; i <= cpu_last_tlb_thread_sibling(cpu);
- i += cpu_tlb_thread_sibling_step())
- if (cpumask_test_cpu(i, cpu_in_guest))
+ i += cpu_tlb_thread_sibling_step()) {
+ struct kvm *running = *per_cpu_ptr(&cpu_in_guest, i);
+
+ if (running == kvm)
smp_call_function_single(i, do_nothing, NULL, 1);
+ }
}
static void do_migrate_away_vcpu(void *arg)
@@ -3072,7 +3075,6 @@ static void kvmppc_start_thread(struct kvm_vcpu *vcpu, struct kvmppc_vcore *vc)
{
int cpu;
struct paca_struct *tpaca;
- struct kvm *kvm = vc->kvm;
cpu = vc->pcpu;
if (vcpu) {
@@ -3083,7 +3085,6 @@ static void kvmppc_start_thread(struct kvm_vcpu *vcpu, struct kvmppc_vcore *vc)
cpu += vcpu->arch.ptid;
vcpu->cpu = vc->pcpu;
vcpu->arch.thread_cpu = cpu;
- cpumask_set_cpu(cpu, &kvm->arch.cpu_in_guest);
}
tpaca = paca_ptrs[cpu];
tpaca->kvm_hstate.kvm_vcpu = vcpu;
@@ -3801,7 +3802,6 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
kvmppc_release_hwthread(pcpu + i);
if (sip && sip->napped[i])
kvmppc_ipi_thread(pcpu + i);
- cpumask_clear_cpu(pcpu + i, &vc->kvm->arch.cpu_in_guest);
}
spin_unlock(&vc->lock);
@@ -3968,8 +3968,14 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
}
} else {
+ struct kvm *kvm = vcpu->kvm;
+
kvmppc_xive_push_vcpu(vcpu);
+
+ __this_cpu_write(cpu_in_guest, kvm);
trap = kvmhv_vcpu_entry_p9(vcpu, time_limit, lpcr, tb);
+ __this_cpu_write(cpu_in_guest, NULL);
+
if (trap == BOOK3S_INTERRUPT_SYSCALL && !vcpu->arch.nested &&
!(vcpu->arch.shregs.msr & MSR_PR)) {
unsigned long req = kvmppc_get_gpr(vcpu, 3);
@@ -3994,7 +4000,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
}
kvmppc_xive_pull_vcpu(vcpu);
- if (kvm_is_radix(vcpu->kvm))
+ if (kvm_is_radix(kvm))
vcpu->arch.slb_max = 0;
}
@@ -4459,8 +4465,6 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
powerpc_local_irq_pmu_restore(flags);
- cpumask_clear_cpu(pcpu, &kvm->arch.cpu_in_guest);
-
preempt_enable();
/*
--
2.23.0
^ permalink raw reply related
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