* Re: [PATCH v2] printk: Add a short description string to kmsg_dump()
From: Petr Mladek @ 2024-07-03 8:22 UTC (permalink / raw)
To: Jocelyn Falempe
Cc: Kees Cook, Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Tony Luck,
Guilherme G. Piccoli, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <10ea2ea1-e692-443e-8b48-ce9884e8b942@redhat.com>
On Wed 2024-07-03 09:57:26, Jocelyn Falempe wrote:
>
>
> On 02/07/2024 22:29, Kees Cook wrote:
> > On Tue, Jul 02, 2024 at 02:26:04PM +0200, Jocelyn Falempe wrote:
> > > kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
> > > callback.
> > > This patch adds a new struct kmsg_dump_detail, that will hold the
> > > reason and description, and pass it to the dump() callback.
> >
> > Thanks! I like this much better. :)
> >
> > >
> > > To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
> > > function and a macro for backward compatibility.
> > >
> > > I've written this for drm_panic, but it can be useful for other
> > > kmsg_dumper.
> > > It allows to see the panic reason, like "sysrq triggered crash"
> > > or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
> > >
> > > v2:
> > > * Use a struct kmsg_dump_detail to hold the reason and description
> > > pointer, for more flexibility if we want to add other parameters.
> > > (Kees Cook)
> > > * Fix powerpc/nvram_64 build, as I didn't update the forward
> > > declaration of oops_to_nvram()
> >
> > The versioning history commonly goes after the "---".
>
> ok, I was not aware of this.
> >
> > > [...]
> > > diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
> > > index 906521c2329c..65f5a47727bc 100644
> > > --- a/include/linux/kmsg_dump.h
> > > +++ b/include/linux/kmsg_dump.h
> > > @@ -39,6 +39,17 @@ struct kmsg_dump_iter {
> > > u64 next_seq;
> > > };
> > > +/**
> > > + *struct kmsg_dump_detail - kernel crash detail
> >
> > Is kern-doc happy with this? I think there is supposed to be a space
> > between the "*" and the first word:
> >
> > /**
> > * struct kmsg...
> >
> >
> Good catch, yes there is a space missing.
>
> I just checked with "make htmldocs", and in fact include/linux/kmsg_dump.h
> is not indexed for kernel documentation.
> And you can't find the definition of struct kmsg_dumper in the online doc.
> https://www.kernel.org/doc/html/latest/search.html?q=kmsg_dumper
>
> > Otherwise looks good to me!
> >
>
> Thanks.
>
> As this patch touches different subsystems, do you know on which tree it
> should land ?
Andrew usually takes patches against kernel/panic.c.
Or you could take it via the DRM tree, especially if you already have the code
using the string.
Also I could take it via the printk tree. The only complication is
that I am going to be away the following two weeks and would come
back in the middle of the merge window. I do not expect much problems
with this change but...
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v2] printk: Add a short description string to kmsg_dump()
From: Petr Mladek @ 2024-07-03 8:12 UTC (permalink / raw)
To: Jocelyn Falempe
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <20240702122639.248110-1-jfalempe@redhat.com>
On Tue 2024-07-02 14:26:04, Jocelyn Falempe wrote:
> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
> callback.
> This patch adds a new struct kmsg_dump_detail, that will hold the
> reason and description, and pass it to the dump() callback.
>
> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
> function and a macro for backward compatibility.
>
> I've written this for drm_panic, but it can be useful for other
> kmsg_dumper.
> It allows to see the panic reason, like "sysrq triggered crash"
> or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
>
> v2:
> * Use a struct kmsg_dump_detail to hold the reason and description
> pointer, for more flexibility if we want to add other parameters.
> (Kees Cook)
> * Fix powerpc/nvram_64 build, as I didn't update the forward
> declaration of oops_to_nvram()
>
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Looks good to me. With the minor fixes suggested by Kees:
Acked-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v3] PCI: hv: Return zero, not garbage, when reading PCI_INTERRUPT_PIN
From: Krzysztof Wilczyński @ 2024-07-03 8:12 UTC (permalink / raw)
To: Wei Liu
Cc: Linux on Hyper-V List, stable, Michael Kelley, K. Y. Srinivasan,
Haiyang Zhang, Dexuan Cui, Lorenzo Pieralisi, Rob Herring,
Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
In-Reply-To: <ZoTZTvL-SKxZEmu5@liuwe-devbox-debian-v2>
Hello,
> > The intent of the code snippet is to always return 0 for both
> > PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
> >
> > The check misses PCI_INTERRUPT_PIN. This patch fixes that.
> >
> > This is discovered by this call in VFIO:
> >
> > pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
> >
> > The old code does not set *val to 0 because it misses the check for
> > PCI_INTERRUPT_PIN. Garbage is returned in that case.
[...]
>
> Bjorn & other PCI maintainers, do you want to pick this up via your
> tree?
>
> I can pick this up via the hyperv tree if you prefer.
We will pick this up. No worries.
Krzysztof
^ permalink raw reply
* Re: [PATCH v2] printk: Add a short description string to kmsg_dump()
From: Jocelyn Falempe @ 2024-07-03 7:57 UTC (permalink / raw)
To: Kees Cook
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Tony Luck,
Guilherme G. Piccoli, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <202407021326.E75B8EA@keescook>
On 02/07/2024 22:29, Kees Cook wrote:
> On Tue, Jul 02, 2024 at 02:26:04PM +0200, Jocelyn Falempe wrote:
>> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
>> callback.
>> This patch adds a new struct kmsg_dump_detail, that will hold the
>> reason and description, and pass it to the dump() callback.
>
> Thanks! I like this much better. :)
>
>>
>> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
>> function and a macro for backward compatibility.
>>
>> I've written this for drm_panic, but it can be useful for other
>> kmsg_dumper.
>> It allows to see the panic reason, like "sysrq triggered crash"
>> or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
>>
>> v2:
>> * Use a struct kmsg_dump_detail to hold the reason and description
>> pointer, for more flexibility if we want to add other parameters.
>> (Kees Cook)
>> * Fix powerpc/nvram_64 build, as I didn't update the forward
>> declaration of oops_to_nvram()
>
> The versioning history commonly goes after the "---".
ok, I was not aware of this.
>
>> [...]
>> diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
>> index 906521c2329c..65f5a47727bc 100644
>> --- a/include/linux/kmsg_dump.h
>> +++ b/include/linux/kmsg_dump.h
>> @@ -39,6 +39,17 @@ struct kmsg_dump_iter {
>> u64 next_seq;
>> };
>>
>> +/**
>> + *struct kmsg_dump_detail - kernel crash detail
>
> Is kern-doc happy with this? I think there is supposed to be a space
> between the "*" and the first word:
>
> /**
> * struct kmsg...
>
>
Good catch, yes there is a space missing.
I just checked with "make htmldocs", and in fact
include/linux/kmsg_dump.h is not indexed for kernel documentation.
And you can't find the definition of struct kmsg_dumper in the online doc.
https://www.kernel.org/doc/html/latest/search.html?q=kmsg_dumper
> Otherwise looks good to me!
>
Thanks.
As this patch touches different subsystems, do you know on which tree it
should land ?
--
Jocelyn
^ permalink raw reply
* Re: [PATCH v3] PCI: hv: Return zero, not garbage, when reading PCI_INTERRUPT_PIN
From: Wei Liu @ 2024-07-03 4:53 UTC (permalink / raw)
To: Linux on Hyper-V List
Cc: Wei Liu, stable, Michael Kelley, K. Y. Srinivasan, Haiyang Zhang,
Dexuan Cui, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
In-Reply-To: <20240701202606.129606-1-wei.liu@kernel.org>
On Mon, Jul 01, 2024 at 08:26:05PM +0000, Wei Liu wrote:
> The intent of the code snippet is to always return 0 for both
> PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
>
> The check misses PCI_INTERRUPT_PIN. This patch fixes that.
>
> This is discovered by this call in VFIO:
>
> pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
>
> The old code does not set *val to 0 because it misses the check for
> PCI_INTERRUPT_PIN. Garbage is returned in that case.
>
> Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
> Cc: stable@kernel.org
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Bjorn & other PCI maintainers, do you want to pick this up via your
tree?
I can pick this up via the hyperv tree if you prefer.
Thanks,
Wei.
> ---
> v3:
> * Change commit subject line and message per Bjorn's suggestion
> v2:
> * Change the commit subject line and message
> * Change the code according to feedback
> ---
> drivers/pci/controller/pci-hyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 5992280e8110..cdd5be16021d 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1130,8 +1130,8 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
> PCI_CAPABILITY_LIST) {
> /* ROM BARs are unimplemented */
> *val = 0;
> - } else if (where >= PCI_INTERRUPT_LINE && where + size <=
> - PCI_INTERRUPT_PIN) {
> + } else if ((where >= PCI_INTERRUPT_LINE && where + size <= PCI_INTERRUPT_PIN) ||
> + (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
> /*
> * Interrupt Line and Interrupt PIN are hard-wired to zero
> * because this front-end only supports message-signaled
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v2] printk: Add a short description string to kmsg_dump()
From: Kees Cook @ 2024-07-02 20:29 UTC (permalink / raw)
To: Jocelyn Falempe
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Tony Luck,
Guilherme G. Piccoli, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <20240702122639.248110-1-jfalempe@redhat.com>
On Tue, Jul 02, 2024 at 02:26:04PM +0200, Jocelyn Falempe wrote:
> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
> callback.
> This patch adds a new struct kmsg_dump_detail, that will hold the
> reason and description, and pass it to the dump() callback.
Thanks! I like this much better. :)
>
> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
> function and a macro for backward compatibility.
>
> I've written this for drm_panic, but it can be useful for other
> kmsg_dumper.
> It allows to see the panic reason, like "sysrq triggered crash"
> or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
>
> v2:
> * Use a struct kmsg_dump_detail to hold the reason and description
> pointer, for more flexibility if we want to add other parameters.
> (Kees Cook)
> * Fix powerpc/nvram_64 build, as I didn't update the forward
> declaration of oops_to_nvram()
The versioning history commonly goes after the "---".
> [...]
> diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
> index 906521c2329c..65f5a47727bc 100644
> --- a/include/linux/kmsg_dump.h
> +++ b/include/linux/kmsg_dump.h
> @@ -39,6 +39,17 @@ struct kmsg_dump_iter {
> u64 next_seq;
> };
>
> +/**
> + *struct kmsg_dump_detail - kernel crash detail
Is kern-doc happy with this? I think there is supposed to be a space
between the "*" and the first word:
/**
* struct kmsg...
Otherwise looks good to me!
--
Kees Cook
^ permalink raw reply
* [PATCH v2] printk: Add a short description string to kmsg_dump()
From: Jocelyn Falempe @ 2024-07-02 12:26 UTC (permalink / raw)
To: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jocelyn Falempe, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
callback.
This patch adds a new struct kmsg_dump_detail, that will hold the
reason and description, and pass it to the dump() callback.
To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
function and a macro for backward compatibility.
I've written this for drm_panic, but it can be useful for other
kmsg_dumper.
It allows to see the panic reason, like "sysrq triggered crash"
or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
v2:
* Use a struct kmsg_dump_detail to hold the reason and description
pointer, for more flexibility if we want to add other parameters.
(Kees Cook)
* Fix powerpc/nvram_64 build, as I didn't update the forward
declaration of oops_to_nvram()
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
arch/powerpc/kernel/nvram_64.c | 8 ++++----
arch/powerpc/platforms/powernv/opal-kmsg.c | 4 ++--
arch/um/kernel/kmsg_dump.c | 2 +-
drivers/gpu/drm/drm_panic.c | 4 ++--
drivers/hv/hv_common.c | 4 ++--
drivers/mtd/mtdoops.c | 2 +-
fs/pstore/platform.c | 10 +++++-----
include/linux/kmsg_dump.h | 22 +++++++++++++++++++---
kernel/panic.c | 2 +-
kernel/printk/printk.c | 11 ++++++++---
10 files changed, 45 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index e385d3164648..f9c6568a9137 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -73,7 +73,7 @@ static const char *nvram_os_partitions[] = {
};
static void oops_to_nvram(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason);
+ struct kmsg_dump_detail *detail);
static struct kmsg_dumper nvram_kmsg_dumper = {
.dump = oops_to_nvram
@@ -643,7 +643,7 @@ void __init nvram_init_oops_partition(int rtas_partition_exists)
* partition. If that's too much, go back and capture uncompressed text.
*/
static void oops_to_nvram(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
static unsigned int oops_count = 0;
@@ -655,7 +655,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
unsigned int err_type = ERR_TYPE_KERNEL_PANIC_GZ;
int rc = -1;
- switch (reason) {
+ switch (detail->reason) {
case KMSG_DUMP_SHUTDOWN:
/* These are almost always orderly shutdowns. */
return;
@@ -671,7 +671,7 @@ static void oops_to_nvram(struct kmsg_dumper *dumper,
break;
default:
pr_err("%s: ignoring unrecognized KMSG_DUMP_* reason %d\n",
- __func__, (int) reason);
+ __func__, (int) detail->reason);
return;
}
diff --git a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c
index 6c3bc4b4da98..bb4218fa796e 100644
--- a/arch/powerpc/platforms/powernv/opal-kmsg.c
+++ b/arch/powerpc/platforms/powernv/opal-kmsg.c
@@ -20,13 +20,13 @@
* message, it just ensures that OPAL completely flushes the console buffer.
*/
static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
/*
* Outside of a panic context the pollers will continue to run,
* so we don't need to do any special flushing.
*/
- if (reason != KMSG_DUMP_PANIC)
+ if (detail->reason != KMSG_DUMP_PANIC)
return;
opal_flush_console(0);
diff --git a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
index 4382cf02a6d1..419021175272 100644
--- a/arch/um/kernel/kmsg_dump.c
+++ b/arch/um/kernel/kmsg_dump.c
@@ -8,7 +8,7 @@
#include <os.h>
static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
static struct kmsg_dump_iter iter;
static DEFINE_SPINLOCK(lock);
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 948aed00595e..8794c7f6c0ee 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -655,11 +655,11 @@ static struct drm_plane *to_drm_plane(struct kmsg_dumper *kd)
return container_of(kd, struct drm_plane, kmsg_panic);
}
-static void drm_panic(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason)
+static void drm_panic(struct kmsg_dumper *dumper, struct kmsg_dump_detail *detail)
{
struct drm_plane *plane = to_drm_plane(dumper);
- if (reason == KMSG_DUMP_PANIC)
+ if (detail->reason == KMSG_DUMP_PANIC)
draw_panic_plane(plane);
}
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 9c452bfbd571..7a35c82976e0 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -207,13 +207,13 @@ static int hv_die_panic_notify_crash(struct notifier_block *self,
* buffer and call into Hyper-V to transfer the data.
*/
static void hv_kmsg_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
struct kmsg_dump_iter iter;
size_t bytes_written;
/* We are only interested in panics. */
- if (reason != KMSG_DUMP_PANIC || !sysctl_record_panic_msg)
+ if (detail->reason != KMSG_DUMP_PANIC || !sysctl_record_panic_msg)
return;
/*
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 2f11585b5613..86d49db9196d 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -298,7 +298,7 @@ static void find_next_position(struct mtdoops_context *cxt)
}
static void mtdoops_do_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
struct mtdoops_context *cxt = container_of(dumper,
struct mtdoops_context, dump);
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 3497ede88aa0..9c6b7c97fa3c 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -275,7 +275,7 @@ void pstore_record_init(struct pstore_record *record,
* end of the buffer.
*/
static void pstore_dump(struct kmsg_dumper *dumper,
- enum kmsg_dump_reason reason)
+ struct kmsg_dump_detail *detail)
{
struct kmsg_dump_iter iter;
unsigned long total = 0;
@@ -285,9 +285,9 @@ static void pstore_dump(struct kmsg_dumper *dumper,
int saved_ret = 0;
int ret;
- why = kmsg_dump_reason_str(reason);
+ why = kmsg_dump_reason_str(detail->reason);
- if (pstore_cannot_block_path(reason)) {
+ if (pstore_cannot_block_path(detail->reason)) {
if (!spin_trylock_irqsave(&psinfo->buf_lock, flags)) {
pr_err("dump skipped in %s path because of concurrent dump\n",
in_nmi() ? "NMI" : why);
@@ -311,7 +311,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
pstore_record_init(&record, psinfo);
record.type = PSTORE_TYPE_DMESG;
record.count = oopscount;
- record.reason = reason;
+ record.reason = detail->reason;
record.part = part;
record.buf = psinfo->buf;
@@ -352,7 +352,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
}
ret = psinfo->write(&record);
- if (ret == 0 && reason == KMSG_DUMP_OOPS) {
+ if (ret == 0 && detail->reason == KMSG_DUMP_OOPS) {
pstore_new_entry = 1;
pstore_timer_kick();
} else {
diff --git a/include/linux/kmsg_dump.h b/include/linux/kmsg_dump.h
index 906521c2329c..65f5a47727bc 100644
--- a/include/linux/kmsg_dump.h
+++ b/include/linux/kmsg_dump.h
@@ -39,6 +39,17 @@ struct kmsg_dump_iter {
u64 next_seq;
};
+/**
+ *struct kmsg_dump_detail - kernel crash detail
+ * @reason: reason for the crash, see kmsg_dump_reason.
+ * @description: optional short string, to provide additional information.
+ */
+
+struct kmsg_dump_detail {
+ enum kmsg_dump_reason reason;
+ const char *description;
+};
+
/**
* struct kmsg_dumper - kernel crash message dumper structure
* @list: Entry in the dumper list (private)
@@ -49,13 +60,13 @@ struct kmsg_dump_iter {
*/
struct kmsg_dumper {
struct list_head list;
- void (*dump)(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason);
+ void (*dump)(struct kmsg_dumper *dumper, struct kmsg_dump_detail *detail);
enum kmsg_dump_reason max_reason;
bool registered;
};
#ifdef CONFIG_PRINTK
-void kmsg_dump(enum kmsg_dump_reason reason);
+void kmsg_dump_desc(enum kmsg_dump_reason reason, const char *desc);
bool kmsg_dump_get_line(struct kmsg_dump_iter *iter, bool syslog,
char *line, size_t size, size_t *len);
@@ -71,7 +82,7 @@ int kmsg_dump_unregister(struct kmsg_dumper *dumper);
const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason);
#else
-static inline void kmsg_dump(enum kmsg_dump_reason reason)
+static inline void kmsg_dump_desc(enum kmsg_dump_reason reason, const char *desc)
{
}
@@ -107,4 +118,9 @@ static inline const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason)
}
#endif
+static inline void kmsg_dump(enum kmsg_dump_reason reason)
+{
+ kmsg_dump_desc(reason, NULL);
+}
+
#endif /* _LINUX_KMSG_DUMP_H */
diff --git a/kernel/panic.c b/kernel/panic.c
index 0843a275531a..0a8b29c44f3c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -378,7 +378,7 @@ void panic(const char *fmt, ...)
panic_print_sys_info(false);
- kmsg_dump(KMSG_DUMP_PANIC);
+ kmsg_dump_desc(KMSG_DUMP_PANIC, buf);
/*
* If you doubt kdump always works fine in any situation,
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 5dcc05e1aa56..19bc414be5f0 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4272,16 +4272,21 @@ const char *kmsg_dump_reason_str(enum kmsg_dump_reason reason)
EXPORT_SYMBOL_GPL(kmsg_dump_reason_str);
/**
- * kmsg_dump - dump kernel log to kernel message dumpers.
+ * kmsg_dump_desc - dump kernel log to kernel message dumpers.
* @reason: the reason (oops, panic etc) for dumping
+ * @desc: a short string to describe what caused the panic or oops. Can be NULL
+ * if no additional description is available.
*
* Call each of the registered dumper's dump() callback, which can
* retrieve the kmsg records with kmsg_dump_get_line() or
* kmsg_dump_get_buffer().
*/
-void kmsg_dump(enum kmsg_dump_reason reason)
+void kmsg_dump_desc(enum kmsg_dump_reason reason, const char *desc)
{
struct kmsg_dumper *dumper;
+ struct kmsg_dump_detail detail = {
+ .reason = reason,
+ .description = desc};
rcu_read_lock();
list_for_each_entry_rcu(dumper, &dump_list, list) {
@@ -4299,7 +4304,7 @@ void kmsg_dump(enum kmsg_dump_reason reason)
continue;
/* invoke dumper which will iterate over records */
- dumper->dump(dumper, reason);
+ dumper->dump(dumper, &detail);
}
rcu_read_unlock();
}
base-commit: 82e4255305c554b0bb18b7ccf2db86041b4c8b6e
--
2.45.2
^ permalink raw reply related
* [PATCH v2] tools: hv: lsvmbus: change shebang to use python3
From: Anthony Nandaa @ 2024-07-02 10:22 UTC (permalink / raw)
To: linux-hyperv, decui, mhklinux, wei.liu; +Cc: kys, Anthony Nandaa
In many modern Linux distros, running `lsvmbus` returns the error:
```
/usr/bin/env: 'python': No such file or directory
```
because 'python' doesn't point anywhere.
Now that python2 has reached EOL as of January 1, 2020 and is no longer
maintained[1], these distros have python3 instead.
Also, the script isn't executable by default because the permissions are
set to mode 644.
Fix this by updating the shebang in the `lsvmbus` to use python3 instead
of python. Also fix the permissions to be 755 so that is executable by
default, which matches other similar scripts in `tools/hv`.
The script is also tested and verified that is compatible with
python3.
[1] https://www.python.org/doc/sunset-python-2/
Signed-off-by: Anthony Nandaa <profnandaa@gmail.com>
Reviewed-by: Wei Liu <wei.liu@kernel.org>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
v2:
* change the commit message body to conform to guidelines.
---
tools/hv/lsvmbus | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
mode change 100644 => 100755 tools/hv/lsvmbus
diff --git a/tools/hv/lsvmbus b/tools/hv/lsvmbus
old mode 100644
new mode 100755
index 55e7374bade0..23dcd8e705be
--- a/tools/hv/lsvmbus
+++ b/tools/hv/lsvmbus
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0
import os
--
2.39.4
^ permalink raw reply related
* Re: [PATCH] tools: hv: lsvmbus: change shebang to use python3
From: Anthony Nandaa @ 2024-07-02 9:32 UTC (permalink / raw)
To: Wei Liu; +Cc: linux-hyperv, decui, mhklinux, kys
In-Reply-To: <ZoMUVE2cuHDkcdbs@liuwe-devbox-debian-v2>
Sure, the script is compatible with Python3. I have tested it with the
two command options (-v, -vv) and runs okay.
Also, I have run it through the ast module [1] just to make sure that
there are no any syntax issues.
Thanks for taking a look!
[1] https://docs.python.org/3/library/ast.html
Regards,
Nandaa
On Mon, 1 Jul 2024 at 23:41, Wei Liu <wei.liu@kernel.org> wrote:
>
> On Mon, Jul 01, 2024 at 08:35:55AM +0000, Anthony Nandaa wrote:
> > This patch updates the shebang in the lsvmbus tool to use python3
> > instead of python. The change is necessary because Python 2 has
> > reached its end of life as of January 1, 2020, and is no longer
> > maintained[1]. Many modern systems do not have python pointing to
> > Python 2, and instead use python3.
> >
> > By explicitly using python3, we ensure compatibility with modern
> > systems since Python 2 is no longer being shipped by default.
> >
> > This change also updates the file permissions to make the script
> > executable, so that the script runs out of the box.
> > Also, similar scripts within `tools/hv` have mode `755`:
> >
> > ```
> > -rwxr-xr-x 1 labuser labuser 930 Jun 28 16:15 hv_get_dhcp_info.sh
> > -rwxr-xr-x 1 labuser labuser 622 Jun 28 16:15 hv_get_dns_info.sh
> > -rwxr-xr-x 1 labuser labuser 1888 Jun 28 16:15 hv_set_ifconfig.sh
> > ```
> >
> > Before fix, this is what you get when you attempt to run `lsvmbus`:
> > ```
> > /usr/bin/env: ‘python’: No such file or directory
> > ```
> >
> > [1] https://www.python.org/doc/sunset-python-2/
> >
> > Signed-off-by: Anthony Nandaa <profnandaa@gmail.com>
>
> Have you checked if the scripts are compatible with python3?
>
> Thanks,
> Wei.
>
> > ---
> > tools/hv/lsvmbus | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > mode change 100644 => 100755 tools/hv/lsvmbus
> >
> > diff --git a/tools/hv/lsvmbus b/tools/hv/lsvmbus
> > old mode 100644
> > new mode 100755
> > index 55e7374bade0..23dcd8e705be
> > --- a/tools/hv/lsvmbus
> > +++ b/tools/hv/lsvmbus
> > @@ -1,4 +1,4 @@
> > -#!/usr/bin/env python
> > +#!/usr/bin/env python3
> > # SPDX-License-Identifier: GPL-2.0
> >
> > import os
> > --
> > 2.33.8
> >
> >
--
___
Nandaa Anthony // nandaa.dev
^ permalink raw reply
* Re: [PATCH] tools: hv: lsvmbus: change shebang to use python3
From: Anthony Nandaa @ 2024-07-02 9:29 UTC (permalink / raw)
To: Michael Kelley
Cc: linux-hyperv@vger.kernel.org, decui@microsoft.com,
kys@microsoft.com
In-Reply-To: <SN6PR02MB4157668B1A29D9E0F5E01C77D4DC2@SN6PR02MB4157.namprd02.prod.outlook.com>
On Tue, 2 Jul 2024 at 06:15, Michael Kelley <mhklinux@outlook.com> wrote:
>
> From: Anthony Nandaa <profnandaa@gmail.com> Sent: Monday, July 1, 2024 1:36 AM
> >
> > This patch updates the shebang in the lsvmbus tool to use python3
> > instead of python. The change is necessary because Python 2 has
> > reached its end of life as of January 1, 2020, and is no longer
> > maintained[1]. Many modern systems do not have python pointing to
> > Python 2, and instead use python3.
> >
> > By explicitly using python3, we ensure compatibility with modern
> > systems since Python 2 is no longer being shipped by default.
> >
> > This change also updates the file permissions to make the script
> > executable, so that the script runs out of the box.
> > Also, similar scripts within `tools/hv` have mode `755`:
> >
> > ```
> > -rwxr-xr-x 1 labuser labuser 930 Jun 28 16:15 hv_get_dhcp_info.sh
> > -rwxr-xr-x 1 labuser labuser 622 Jun 28 16:15 hv_get_dns_info.sh
> > -rwxr-xr-x 1 labuser labuser 1888 Jun 28 16:15 hv_set_ifconfig.sh
> > ```
> >
> > Before fix, this is what you get when you attempt to run `lsvmbus`:
> > ```
> > /usr/bin/env: 'python': No such file or directory
> > ```
> >
>
> A note about commit message style. The guidelines in
> Documentation/process/submitting-patches.rst specifically say to
> use imperative mood and avoid "This patch" (and by extension,
> "This change"). For a patch that is fixing a problem, I usually
> describe the problem first, and then start a new paragraph with
> "Fix this problem by .....". So for your patch, I would suggest
> something like:
>
> In many modern Linux distros, running "lsvmbus" returns the error:
>
> /usr/bin/env: 'python': No such file or directory
>
> because 'python' doesn't point anywhere. Now that python2 has
> reached end of life as of January 1, 2020 and is no longer
> maintained[1], these distros have python3 instead. Also, the script
> isn't executable by default because the permissions are set to
> mode 644.
>
> Fix this by updating the shebang in the lsvmbus to use python3
> instead of python. Also fix the permissions to be 755 so that it is
> executable by default, which matches other similar scripts in tools/hv.
>
> Michael
Thanks Michael for the guidance. This is well noted and I'm going to fix
in my revision.
Regards,
Nandaa
^ permalink raw reply
* RE: [PATCH] tools: hv: lsvmbus: change shebang to use python3
From: Michael Kelley @ 2024-07-02 3:15 UTC (permalink / raw)
To: Anthony Nandaa, linux-hyperv@vger.kernel.org, decui@microsoft.com
Cc: kys@microsoft.com
In-Reply-To: <20240701083554.11967-1-profnandaa@gmail.com>
From: Anthony Nandaa <profnandaa@gmail.com> Sent: Monday, July 1, 2024 1:36 AM
>
> This patch updates the shebang in the lsvmbus tool to use python3
> instead of python. The change is necessary because Python 2 has
> reached its end of life as of January 1, 2020, and is no longer
> maintained[1]. Many modern systems do not have python pointing to
> Python 2, and instead use python3.
>
> By explicitly using python3, we ensure compatibility with modern
> systems since Python 2 is no longer being shipped by default.
>
> This change also updates the file permissions to make the script
> executable, so that the script runs out of the box.
> Also, similar scripts within `tools/hv` have mode `755`:
>
> ```
> -rwxr-xr-x 1 labuser labuser 930 Jun 28 16:15 hv_get_dhcp_info.sh
> -rwxr-xr-x 1 labuser labuser 622 Jun 28 16:15 hv_get_dns_info.sh
> -rwxr-xr-x 1 labuser labuser 1888 Jun 28 16:15 hv_set_ifconfig.sh
> ```
>
> Before fix, this is what you get when you attempt to run `lsvmbus`:
> ```
> /usr/bin/env: 'python': No such file or directory
> ```
>
A note about commit message style. The guidelines in
Documentation/process/submitting-patches.rst specifically say to
use imperative mood and avoid "This patch" (and by extension,
"This change"). For a patch that is fixing a problem, I usually
describe the problem first, and then start a new paragraph with
"Fix this problem by .....". So for your patch, I would suggest
something like:
In many modern Linux distros, running "lsvmbus" returns the error:
/usr/bin/env: 'python': No such file or directory
because 'python' doesn't point anywhere. Now that python2 has
reached end of life as of January 1, 2020 and is no longer
maintained[1], these distros have python3 instead. Also, the script
isn't executable by default because the permissions are set to
mode 644.
Fix this by updating the shebang in the lsvmbus to use python3
instead of python. Also fix the permissions to be 755 so that it is
executable by default, which matches other similar scripts in tools/hv.
Michael
^ permalink raw reply
* Re: [PATCH] tools: hv: lsvmbus: change shebang to use python3
From: Wei Liu @ 2024-07-01 20:40 UTC (permalink / raw)
To: Anthony Nandaa; +Cc: linux-hyperv, decui, mhklinux, kys, Wei Liu
In-Reply-To: <20240701083554.11967-1-profnandaa@gmail.com>
On Mon, Jul 01, 2024 at 08:35:55AM +0000, Anthony Nandaa wrote:
> This patch updates the shebang in the lsvmbus tool to use python3
> instead of python. The change is necessary because Python 2 has
> reached its end of life as of January 1, 2020, and is no longer
> maintained[1]. Many modern systems do not have python pointing to
> Python 2, and instead use python3.
>
> By explicitly using python3, we ensure compatibility with modern
> systems since Python 2 is no longer being shipped by default.
>
> This change also updates the file permissions to make the script
> executable, so that the script runs out of the box.
> Also, similar scripts within `tools/hv` have mode `755`:
>
> ```
> -rwxr-xr-x 1 labuser labuser 930 Jun 28 16:15 hv_get_dhcp_info.sh
> -rwxr-xr-x 1 labuser labuser 622 Jun 28 16:15 hv_get_dns_info.sh
> -rwxr-xr-x 1 labuser labuser 1888 Jun 28 16:15 hv_set_ifconfig.sh
> ```
>
> Before fix, this is what you get when you attempt to run `lsvmbus`:
> ```
> /usr/bin/env: ‘python’: No such file or directory
> ```
>
> [1] https://www.python.org/doc/sunset-python-2/
>
> Signed-off-by: Anthony Nandaa <profnandaa@gmail.com>
Have you checked if the scripts are compatible with python3?
Thanks,
Wei.
> ---
> tools/hv/lsvmbus | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> mode change 100644 => 100755 tools/hv/lsvmbus
>
> diff --git a/tools/hv/lsvmbus b/tools/hv/lsvmbus
> old mode 100644
> new mode 100755
> index 55e7374bade0..23dcd8e705be
> --- a/tools/hv/lsvmbus
> +++ b/tools/hv/lsvmbus
> @@ -1,4 +1,4 @@
> -#!/usr/bin/env python
> +#!/usr/bin/env python3
> # SPDX-License-Identifier: GPL-2.0
>
> import os
> --
> 2.33.8
>
>
^ permalink raw reply
* [PATCH v3] PCI: hv: Return zero, not garbage, when reading PCI_INTERRUPT_PIN
From: Wei Liu @ 2024-07-01 20:26 UTC (permalink / raw)
To: Linux on Hyper-V List
Cc: Wei Liu, stable, Michael Kelley, K. Y. Srinivasan, Haiyang Zhang,
Dexuan Cui, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
The intent of the code snippet is to always return 0 for both
PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
The check misses PCI_INTERRUPT_PIN. This patch fixes that.
This is discovered by this call in VFIO:
pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
The old code does not set *val to 0 because it misses the check for
PCI_INTERRUPT_PIN. Garbage is returned in that case.
Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
Cc: stable@kernel.org
Signed-off-by: Wei Liu <wei.liu@kernel.org>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
v3:
* Change commit subject line and message per Bjorn's suggestion
v2:
* Change the commit subject line and message
* Change the code according to feedback
---
drivers/pci/controller/pci-hyperv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 5992280e8110..cdd5be16021d 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1130,8 +1130,8 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
PCI_CAPABILITY_LIST) {
/* ROM BARs are unimplemented */
*val = 0;
- } else if (where >= PCI_INTERRUPT_LINE && where + size <=
- PCI_INTERRUPT_PIN) {
+ } else if ((where >= PCI_INTERRUPT_LINE && where + size <= PCI_INTERRUPT_PIN) ||
+ (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
/*
* Interrupt Line and Interrupt PIN are hard-wired to zero
* because this front-end only supports message-signaled
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
From: Wei Liu @ 2024-07-01 20:01 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Wei Liu, Linux on Hyper-V List, stable, K. Y. Srinivasan,
Haiyang Zhang, Dexuan Cui, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
In-Reply-To: <20240701172053.GA10100@bhelgaas>
On Mon, Jul 01, 2024 at 12:20:53PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 01, 2024 at 06:16:18AM +0000, Wei Liu wrote:
> > On Wed, Jun 26, 2024 at 10:10:39AM -0500, Bjorn Helgaas wrote:
> > > 1) Capitalize subject to match history
> >
> > What do you mean here? I got the "PCI: hv: ..." format from recent
> > commits. "PCI" is capitalized. You want to to capitalize "fix"?
>
> Yes. Look at the history:
>
> $ git log --oneline --no-merges drivers/pci/controller/pci-hyperv.c
> b5ff74c1ef50 PCI: hv: Fix ring buffer size calculation
> 07e8f88568f5 x86/apic: Drop apic::delivery_mode
> f741bcadfe52 PCI: hv: Annotate struct hv_dr_state with __counted_by
> 04bbe863241a PCI: hv: Fix a crash in hv_pci_restore_msi_msg() during hibernation
> 067d6ec7ed5b PCI: hv: Add a per-bus mutex state_lock
> a847234e24d0 Revert "PCI: hv: Fix a timing issue which causes kdump to fail occasionally"
> add9195e69c9 PCI: hv: Remove the useless hv_pcichild_state from struct hv_pci_dev
> 2738d5ab7929 PCI: hv: Fix a race condition in hv_irq_unmask() that can cause panic
> ...
>
> > > 2) Say something more specific than "fix reading ..."
> > >
> > > Apparently this returns garbage in some case where you want to return
> > > zero?
> >
> > Yes. *val is not changed in the old code, so garbage is returned.
> >
> > Here is the updated commit message. I can resend once you confirm you're
> > happy with it.
> >
> > PCI: hv: Fix reading of PCI_INTERRUPT_PIN
>
> Maybe:
>
> PCI: hv: Return zero, not garbage, when reading PCI_INTERRUPT_PIN
>
> > The intent of the code snippet is to always return 0 for both
> > PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
> >
> > The check misses PCI_INTERRUPT_PIN. This patch fixes that.
> >
> > This is discovered by this call in VFIO:
> >
> > pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
> >
> > The old code does not set *val to 0 because it misses the check for
> > PCI_INTERRUPT_PIN. Garbage is returned in this case.
> >
> > Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
> > Cc: stable@kernel.org
> > Signed-off-by: Wei Liu <wei.liu@kernel.org>
>
> Looks fine.
Thanks. I will resend with the updated commit message.
^ permalink raw reply
* Re: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
From: Bjorn Helgaas @ 2024-07-01 17:20 UTC (permalink / raw)
To: Wei Liu
Cc: Linux on Hyper-V List, stable, K. Y. Srinivasan, Haiyang Zhang,
Dexuan Cui, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
In-Reply-To: <ZoJJsolJJcLUYiVG@liuwe-devbox-debian-v2>
On Mon, Jul 01, 2024 at 06:16:18AM +0000, Wei Liu wrote:
> On Wed, Jun 26, 2024 at 10:10:39AM -0500, Bjorn Helgaas wrote:
> > 1) Capitalize subject to match history
>
> What do you mean here? I got the "PCI: hv: ..." format from recent
> commits. "PCI" is capitalized. You want to to capitalize "fix"?
Yes. Look at the history:
$ git log --oneline --no-merges drivers/pci/controller/pci-hyperv.c
b5ff74c1ef50 PCI: hv: Fix ring buffer size calculation
07e8f88568f5 x86/apic: Drop apic::delivery_mode
f741bcadfe52 PCI: hv: Annotate struct hv_dr_state with __counted_by
04bbe863241a PCI: hv: Fix a crash in hv_pci_restore_msi_msg() during hibernation
067d6ec7ed5b PCI: hv: Add a per-bus mutex state_lock
a847234e24d0 Revert "PCI: hv: Fix a timing issue which causes kdump to fail occasionally"
add9195e69c9 PCI: hv: Remove the useless hv_pcichild_state from struct hv_pci_dev
2738d5ab7929 PCI: hv: Fix a race condition in hv_irq_unmask() that can cause panic
...
> > 2) Say something more specific than "fix reading ..."
> >
> > Apparently this returns garbage in some case where you want to return
> > zero?
>
> Yes. *val is not changed in the old code, so garbage is returned.
>
> Here is the updated commit message. I can resend once you confirm you're
> happy with it.
>
> PCI: hv: Fix reading of PCI_INTERRUPT_PIN
Maybe:
PCI: hv: Return zero, not garbage, when reading PCI_INTERRUPT_PIN
> The intent of the code snippet is to always return 0 for both
> PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
>
> The check misses PCI_INTERRUPT_PIN. This patch fixes that.
>
> This is discovered by this call in VFIO:
>
> pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
>
> The old code does not set *val to 0 because it misses the check for
> PCI_INTERRUPT_PIN. Garbage is returned in this case.
>
> Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
> Cc: stable@kernel.org
> Signed-off-by: Wei Liu <wei.liu@kernel.org>
Looks fine.
^ permalink raw reply
* Re: [PATCH] Drivers: hv: vmbus: Add missing check for dma_set_mask in vmbus_device_register()
From: Markus Elfring @ 2024-07-01 9:30 UTC (permalink / raw)
To: Haoxiang Li, linux-hyperv, kernel-janitors, Andrea Parri,
Dexuan Cui, Haiyang Zhang, K. Y. Srinivasan, Michael Kelley,
Wei Liu
Cc: LKML
In-Reply-To: <20240701023059.83616-1-make24@iscas.ac.cn>
> child_device_obj->device cannot perform DMA properly if dma_set_mask()
> returns non-zero. …
Another wording suggestion:
Direct memory access can not be properly performed any more
after a dma_set_mask() call failed.
See also:
https://elixir.bootlin.com/linux/v6.10-rc6/source/kernel/dma/mapping.c#L804
> … child_device_obj->device is not initialized here, so use kfree() to
> free child_device_obj->device.
How did you come to the conclusion that meaningful data processing
would still be possible according to such information?
…
> Signed-off-by: Haoxiang Li <make24@iscas.ac.cn>
I find it interesting that another personal name is presented here.
I noticed that some patches were published with the name “Ma Ke” previously.
How will requirements be resolved for the Developer's Certificate of Origin?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.10-rc6#n398
Would you like to append parentheses to another function name in the summary phrase?
Regards,
Markus
^ permalink raw reply
* [PATCH] tools: hv: lsvmbus: change shebang to use python3
From: Anthony Nandaa @ 2024-07-01 8:35 UTC (permalink / raw)
To: linux-hyperv, decui, mhklinux; +Cc: kys, Anthony Nandaa
This patch updates the shebang in the lsvmbus tool to use python3
instead of python. The change is necessary because Python 2 has
reached its end of life as of January 1, 2020, and is no longer
maintained[1]. Many modern systems do not have python pointing to
Python 2, and instead use python3.
By explicitly using python3, we ensure compatibility with modern
systems since Python 2 is no longer being shipped by default.
This change also updates the file permissions to make the script
executable, so that the script runs out of the box.
Also, similar scripts within `tools/hv` have mode `755`:
```
-rwxr-xr-x 1 labuser labuser 930 Jun 28 16:15 hv_get_dhcp_info.sh
-rwxr-xr-x 1 labuser labuser 622 Jun 28 16:15 hv_get_dns_info.sh
-rwxr-xr-x 1 labuser labuser 1888 Jun 28 16:15 hv_set_ifconfig.sh
```
Before fix, this is what you get when you attempt to run `lsvmbus`:
```
/usr/bin/env: ‘python’: No such file or directory
```
[1] https://www.python.org/doc/sunset-python-2/
Signed-off-by: Anthony Nandaa <profnandaa@gmail.com>
---
tools/hv/lsvmbus | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
mode change 100644 => 100755 tools/hv/lsvmbus
diff --git a/tools/hv/lsvmbus b/tools/hv/lsvmbus
old mode 100644
new mode 100755
index 55e7374bade0..23dcd8e705be
--- a/tools/hv/lsvmbus
+++ b/tools/hv/lsvmbus
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
# SPDX-License-Identifier: GPL-2.0
import os
--
2.33.8
^ permalink raw reply related
* Re: [PATCH v2] PCI: hv: fix reading of PCI_INTERRUPT_PIN
From: Wei Liu @ 2024-07-01 6:16 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Wei Liu, Linux on Hyper-V List, stable, K. Y. Srinivasan,
Haiyang Zhang, Dexuan Cui, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Jake Oshins,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list
In-Reply-To: <20240626151039.GA1466747@bhelgaas>
On Wed, Jun 26, 2024 at 10:10:39AM -0500, Bjorn Helgaas wrote:
> 1) Capitalize subject to match history
What do you mean here? I got the "PCI: hv: ..." format from recent
commits. "PCI" is capitalized. You want to to capitalize "fix"?
> 2) Say something more specific than "fix reading ..."
>
> Apparently this returns garbage in some case where you want to return
> zero?
Yes. *val is not changed in the old code, so garbage is returned.
Here is the updated commit message. I can resend once you confirm you're
happy with it.
PCI: hv: Fix reading of PCI_INTERRUPT_PIN
The intent of the code snippet is to always return 0 for both
PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
The check misses PCI_INTERRUPT_PIN. This patch fixes that.
This is discovered by this call in VFIO:
pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
The old code does not set *val to 0 because it misses the check for
PCI_INTERRUPT_PIN. Garbage is returned in this case.
Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
Cc: stable@kernel.org
Signed-off-by: Wei Liu <wei.liu@kernel.org>
Thanks,
Wei.
>
> On Fri, Jun 21, 2024 at 09:00:18PM +0000, Wei Liu wrote:
> > The intent of the code snippet is to always return 0 for both
> > PCI_INTERRUPT_LINE and PCI_INTERRUPT_PIN.
> >
> > The check misses PCI_INTERRUPT_PIN. This patch fixes that.
> >
> > This is discovered by this call in VFIO:
> >
> > pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
> >
> > The old code does not set *val to 0 because it misses the check for
> > PCI_INTERRUPT_PIN.
> >
> > Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs")
> > Cc: stable@kernel.org
> > Signed-off-by: Wei Liu <wei.liu@kernel.org>
> > ---
> > v2:
> > * Change the commit subject line and message
> > * Change the code according to feedback
> > ---
> > drivers/pci/controller/pci-hyperv.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> > index 5992280e8110..cdd5be16021d 100644
> > --- a/drivers/pci/controller/pci-hyperv.c
> > +++ b/drivers/pci/controller/pci-hyperv.c
> > @@ -1130,8 +1130,8 @@ static void _hv_pcifront_read_config(struct hv_pci_dev *hpdev, int where,
> > PCI_CAPABILITY_LIST) {
> > /* ROM BARs are unimplemented */
> > *val = 0;
> > - } else if (where >= PCI_INTERRUPT_LINE && where + size <=
> > - PCI_INTERRUPT_PIN) {
> > + } else if ((where >= PCI_INTERRUPT_LINE && where + size <= PCI_INTERRUPT_PIN) ||
> > + (where >= PCI_INTERRUPT_PIN && where + size <= PCI_MIN_GNT)) {
> > /*
> > * Interrupt Line and Interrupt PIN are hard-wired to zero
> > * because this front-end only supports message-signaled
> > --
> > 2.43.0
> >
^ permalink raw reply
* RE: [PATCH] Drivers: hv: vmbus: Add missing check for dma_set_mask in vmbus_device_register()
From: Michael Kelley @ 2024-07-01 3:47 UTC (permalink / raw)
To: Haoxiang Li, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, parri.andrea@gmail.com,
mikelley@microsoft.com
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20240701023059.83616-1-make24@iscas.ac.cn>
From: Haoxiang Li <make24@iscas.ac.cn> Sent: Sunday, June 30, 2024 7:31 PM
>
> child_device_obj->device cannot perform DMA properly if dma_set_mask()
> returns non-zero. Add check for dma_set_mask() and return the error if it
> fails. child_device_obj->device is not initialized here, so use kfree() to
> free child_device_obj->device.
>
> Fixes: 3a5469582c24 ("Drivers: hv: vmbus: Fix initialization of device object in
> vmbus_device_register()")
> Signed-off-by: Haoxiang Li <make24@iscas.ac.cn>
> ---
> drivers/hv/vmbus_drv.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 12a707ab73f8..daa584eaa2af 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1897,7 +1897,13 @@ int vmbus_device_register(struct hv_device
> *child_device_obj)
>
> child_device_obj->device.dma_parms = &child_device_obj->dma_parms;
> child_device_obj->device.dma_mask = &child_device_obj->dma_mask;
> - dma_set_mask(&child_device_obj->device, DMA_BIT_MASK(64));
> + ret = dma_set_mask(&child_device_obj->device, DMA_BIT_MASK(64));
> +
> + if (ret) {
> + pr_err("64-bit DMA enable failed\n");
> + kfree(&child_device_obj->device);
> + return ret;
> + }
I commented on a previous attempt to check the return value from
dma_set_mask(). See [1]. See also commit f92a4b50f0bd that adds
some detail about the error handling in vmbus_device_register(). It's
all rather messy. I have not personally worked through the details, so
I'm not sure of what the right cleanup is. But I don't think it is
sufficient to just do the kfree() as your patch proposes. Or if I'm wrong,
and that *is* sufficient, please provides details on the reasoning
why, and let's make sure to record that for future reference! :-)
Michael
[1] https://lore.kernel.org/linux-hyperv/20230607014310.19850-1-jiasheng@iscas.ac.cn/
^ permalink raw reply
* [PATCH] Drivers: hv: vmbus: Add missing check for dma_set_mask in vmbus_device_register()
From: Haoxiang Li @ 2024-07-01 2:30 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, parri.andrea, mikelley
Cc: linux-hyperv, linux-kernel, Haoxiang Li
child_device_obj->device cannot perform DMA properly if dma_set_mask()
returns non-zero. Add check for dma_set_mask() and return the error if it
fails. child_device_obj->device is not initialized here, so use kfree() to
free child_device_obj->device.
Fixes: 3a5469582c24 ("Drivers: hv: vmbus: Fix initialization of device object in vmbus_device_register()")
Signed-off-by: Haoxiang Li <make24@iscas.ac.cn>
---
drivers/hv/vmbus_drv.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 12a707ab73f8..daa584eaa2af 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1897,7 +1897,13 @@ int vmbus_device_register(struct hv_device *child_device_obj)
child_device_obj->device.dma_parms = &child_device_obj->dma_parms;
child_device_obj->device.dma_mask = &child_device_obj->dma_mask;
- dma_set_mask(&child_device_obj->device, DMA_BIT_MASK(64));
+ ret = dma_set_mask(&child_device_obj->device, DMA_BIT_MASK(64));
+
+ if (ret) {
+ pr_err("64-bit DMA enable failed\n");
+ kfree(&child_device_obj->device);
+ return ret;
+ }
/*
* Register with the LDM. This will kick off the driver/device
--
2.25.1
^ permalink raw reply related
* RE: [PATCH] x86/tdx: Support vmalloc() for tdx_enc_status_changed()
From: Dexuan Cui @ 2024-06-28 19:24 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: x86@kernel.org, linux-coco@lists.linux.dev, ak@linux.intel.com,
arnd@arndb.de, bp@alien8.de, brijesh.singh@amd.com,
dan.j.williams@intel.com, dave.hansen@intel.com,
dave.hansen@linux.intel.com, Haiyang Zhang, hpa@zytor.com,
jane.chu@oracle.com, KY Srinivasan, luto@kernel.org,
mingo@redhat.com, peterz@infradead.org, rostedt@goodmis.org,
sathyanarayanan.kuppuswamy@linux.intel.com, seanjc@google.com,
tglx@linutronix.de, tony.luck@intel.com, wei.liu@kernel.org,
jason, nik.borisov@suse.com, mhklinux,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
Tianyu Lan, rick.p.edgecombe@intel.com, andavis@redhat.com,
Mark Heslin, vkuznets, xiaoyao.li@intel.com,
stable@vger.kernel.org
In-Reply-To: <kjt2m2aqnhmwqgn3ox6bkqtn5qurxawgnx3xyh42pu5sp3mwyj@qwyjttwubfck>
> From: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Sent: Friday, June 28, 2024 3:05 AM
> To: Dexuan Cui <decui@microsoft.com>
> > [...]
> > static bool tdx_enc_status_changed(unsigned long vaddr, int numpages,
> [...]
> This patch collied with kexec changes. tdx_kexec_finish() calls
> tdx_enc_status_changed() after clearing pte, so slow_virt_to_phys()
> crashes on in.
>
> Daxuan, could you check if the fixup below works for you on vmalloc
> addresses?
>
> diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
> index ef8ec2425998..5e455c883bcc 100644
> --- a/arch/x86/coco/tdx/tdx.c
> +++ b/arch/x86/coco/tdx/tdx.c
> @@ -813,8 +813,15 @@ static bool tdx_enc_status_changed(unsigned
> long vaddr, int numpages, bool enc)
> step = PAGE_SIZE;
>
> for (addr = start; addr < end; addr += step) {
> - phys_addr_t start_pa = slow_virt_to_phys((void *)addr);
> - phys_addr_t end_pa = start_pa + step;
> + phys_addr_t start_pa;
> + phys_addr_t end_pa;
> +
> + if (virt_addr_valid(addr))
> + start_pa = __pa(addr);
> + else
> + start_pa = slow_virt_to_phys((void *)addr);
> +
> + end_pa = start_pa + step;
>
> if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
> return false;
> --
> Kiryl Shutsemau / Kirill A. Shutemov
Hi Kirill, your fixup works for me.
BTW, I just realized that virt_addr_valid() returns false for a vmalloc'd address.
Thanks,
Dexuan
^ permalink raw reply
* Re: [PATCH] printk: Add a short description string to kmsg_dump()
From: Kees Cook @ 2024-06-28 16:42 UTC (permalink / raw)
To: Jocelyn Falempe
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Tony Luck,
Guilherme G. Piccoli, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <06899fda-de75-4d44-bda5-dcbb3e35d037@redhat.com>
On Fri, Jun 28, 2024 at 09:13:11AM +0200, Jocelyn Falempe wrote:
> It is present in the kdump log, but before all the register dumps.
> So to retrieve it you need to parse the last 30~40 lines of logs, and search
> for a line starting with "Kernel panic - not syncing".
> https://elixir.bootlin.com/linux/v6.10-rc5/source/kernel/panic.c#L341
> But I think that's a bit messy, and I prefer having a kmsg_dump parameter.
Yeah, I totally agree: it should be easy to access the panic reason. I
just wanted to double-check that from pstore's perspective there wasn't
any "new" information here that should be captured somehow.
Thanks!
--
Kees Cook
^ permalink raw reply
* Re: [PATCH] x86/tdx: Support vmalloc() for tdx_enc_status_changed()
From: Kirill A. Shutemov @ 2024-06-28 10:04 UTC (permalink / raw)
To: Dexuan Cui
Cc: x86, linux-coco, ak, arnd, bp, brijesh.singh, dan.j.williams,
dave.hansen, dave.hansen, haiyangz, hpa, jane.chu, kys, luto,
mingo, peterz, rostedt, sathyanarayanan.kuppuswamy, seanjc, tglx,
tony.luck, wei.liu, Jason, nik.borisov, mhklinux, linux-hyperv,
linux-kernel, Tianyu.Lan, rick.p.edgecombe, andavis, mheslin,
vkuznets, xiaoyao.li, stable
In-Reply-To: <20240521021238.1803-1-decui@microsoft.com>
On Mon, May 20, 2024 at 07:12:38PM -0700, Dexuan Cui wrote:
> @@ -785,15 +799,22 @@ static bool tdx_map_gpa(phys_addr_t start, phys_addr_t end, bool enc)
> */
> static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
> {
> - phys_addr_t start = __pa(vaddr);
> - phys_addr_t end = __pa(vaddr + numpages * PAGE_SIZE);
> + unsigned long start = vaddr;
> + unsigned long end = start + numpages * PAGE_SIZE;
> + unsigned long step = end - start;
> + unsigned long addr;
>
> - if (!tdx_map_gpa(start, end, enc))
> - return false;
> + /* Step through page-by-page for vmalloc() mappings */
> + if (is_vmalloc_addr((void *)vaddr))
> + step = PAGE_SIZE;
>
> - /* shared->private conversion requires memory to be accepted before use */
> - if (enc)
> - return tdx_accept_memory(start, end);
> + for (addr = start; addr < end; addr += step) {
> + phys_addr_t start_pa = slow_virt_to_phys((void *)addr);
> + phys_addr_t end_pa = start_pa + step;
> +
> + if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
> + return false;
> + }
>
> return true;
> }
This patch collied with kexec changes. tdx_kexec_finish() calls
tdx_enc_status_changed() after clearing pte, so slow_virt_to_phys()
crashes on in.
Daxuan, could you check if the fixup below works for you on vmalloc
addresses?
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index ef8ec2425998..5e455c883bcc 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -813,8 +813,15 @@ static bool tdx_enc_status_changed(unsigned long vaddr, int numpages, bool enc)
step = PAGE_SIZE;
for (addr = start; addr < end; addr += step) {
- phys_addr_t start_pa = slow_virt_to_phys((void *)addr);
- phys_addr_t end_pa = start_pa + step;
+ phys_addr_t start_pa;
+ phys_addr_t end_pa;
+
+ if (virt_addr_valid(addr))
+ start_pa = __pa(addr);
+ else
+ start_pa = slow_virt_to_phys((void *)addr);
+
+ end_pa = start_pa + step;
if (!tdx_enc_status_changed_phys(start_pa, end_pa, enc))
return false;
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply related
* Re: [PATCH] printk: Add a short description string to kmsg_dump()
From: Jocelyn Falempe @ 2024-06-28 7:13 UTC (permalink / raw)
To: Kees Cook
Cc: Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Naveen N. Rao, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Tony Luck,
Guilherme G. Piccoli, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Andrew Morton, Jani Nikula,
Greg Kroah-Hartman, Kefeng Wang, Thomas Gleixner, Uros Bizjak,
linuxppc-dev, linux-kernel, dri-devel, linux-hyperv, linux-mtd,
linux-hardening
In-Reply-To: <202406260906.533095B1@keescook>
On 26/06/2024 18:26, Kees Cook wrote:
> On Tue, Jun 25, 2024 at 02:39:29PM +0200, Jocelyn Falempe wrote:
>> kmsg_dump doesn't forward the panic reason string to the kmsg_dumper
>> callback.
>> This patch adds a new parameter "const char *desc" to the kmsg_dumper
>> dump() callback, and update all drivers that are using it.
>>
>> To avoid updating all kmsg_dump() call, it adds a kmsg_dump_desc()
>> function and a macro for backward compatibility.
>>
>> I've written this for drm_panic, but it can be useful for other
>> kmsg_dumper.
>> It allows to see the panic reason, like "sysrq triggered crash"
>> or "VFS: Unable to mount root fs on xxxx" on the drm panic screen.
>
> Seems reasonable. Given the prototype before/after:
>
> dump(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason)
>
> dump(struct kmsg_dumper *dumper, enum kmsg_dump_reason reason,
> const char *desc)
>
> Perhaps this should instead be a struct that the panic fills in? Then
> it'll be easy to adjust the struct in the future:
yes that's a good idea, so it's a bit more flexible.
>
> struct kmsg_dump_detail {
> enum kmsg_dump_reason reason;
> const char *description;
> };
>
> dump(struct kmsg_dumper *dumper, struct kmsg_dump *detail)
>
> This .cocci could do the conversion:
>
>
> @ dump_func @
> identifier DUMPER, CALLBACK;
> @@
>
> struct kmsg_dumper DUMPER = {
> .dump = CALLBACK,
> };
>
> @ detail @
> identifier dump_func.CALLBACK;
> identifier DUMPER, REASON;
> @@
>
> CALLBACK(struct kmsg_dumper *DUMPER,
> - enum kmsg_dump_reason REASON
> + struct kmsg_dump_detail *detail
> )
> {
> <...
> - REASON
> + detail->reason
> ...>
> }
>
>
> Also, just to double-check, doesn't the panic reason show up in the
> kmsg_dump log itself (at the end?) I ask since for pstore, "desc" is
> likely redundant since it's capturing the entire console log.
It is present in the kdump log, but before all the register dumps.
So to retrieve it you need to parse the last 30~40 lines of logs, and
search for a line starting with "Kernel panic - not syncing".
https://elixir.bootlin.com/linux/v6.10-rc5/source/kernel/panic.c#L341
But I think that's a bit messy, and I prefer having a kmsg_dump parameter.
--
Jocelyn
>
> -Kees
>
> Here's the patch from the above cocci:
>
>
> diff -u -p a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -207,13 +207,13 @@ static int hv_die_panic_notify_crash(str
> * buffer and call into Hyper-V to transfer the data.
> */
> static void hv_kmsg_dump(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> + struct kmsg_dump_detail *detail)
> {
> struct kmsg_dump_iter iter;
> size_t bytes_written;
>
> /* We are only interested in panics. */
> - if (reason != KMSG_DUMP_PANIC || !sysctl_record_panic_msg)
> + if (detail->reason != KMSG_DUMP_PANIC || !sysctl_record_panic_msg)
> return;
>
> /*
> diff -u -p a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c
> --- a/arch/powerpc/platforms/powernv/opal-kmsg.c
> +++ b/arch/powerpc/platforms/powernv/opal-kmsg.c
> @@ -20,13 +20,13 @@
> * message, it just ensures that OPAL completely flushes the console buffer.
> */
> static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> + struct kmsg_dump_detail *detail)
> {
> /*
> * Outside of a panic context the pollers will continue to run,
> * so we don't need to do any special flushing.
> */
> - if (reason != KMSG_DUMP_PANIC)
> + if (detail->reason != KMSG_DUMP_PANIC)
> return;
>
> opal_flush_console(0);
> diff -u -p a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -73,7 +73,7 @@ static const char *nvram_os_partitions[]
> };
>
> static void oops_to_nvram(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason);
> + struct kmsg_dump_detail *detail);
>
> static struct kmsg_dumper nvram_kmsg_dumper = {
> .dump = oops_to_nvram
> @@ -643,7 +643,7 @@ void __init nvram_init_oops_partition(in
> * partition. If that's too much, go back and capture uncompressed text.
> */
> static void oops_to_nvram(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> + struct kmsg_dump_detail *detail)
> {
> struct oops_log_info *oops_hdr = (struct oops_log_info *)oops_buf;
> static unsigned int oops_count = 0;
> @@ -655,7 +655,7 @@ static void oops_to_nvram(struct kmsg_du
> unsigned int err_type = ERR_TYPE_KERNEL_PANIC_GZ;
> int rc = -1;
>
> - switch (reason) {
> + switch (detail->reason) {
> case KMSG_DUMP_SHUTDOWN:
> /* These are almost always orderly shutdowns. */
> return;
> @@ -671,7 +671,7 @@ static void oops_to_nvram(struct kmsg_du
> break;
> default:
> pr_err("%s: ignoring unrecognized KMSG_DUMP_* reason %d\n",
> - __func__, (int) reason);
> + __func__, (int) detail->reason);
> return;
> }
>
> warning: detail, node 59: record.reason = ... ;[1,2,21,22,32] in pstore_dump may be inconsistently modified
> warning: detail, node 105: if[1,2,21,22,54] in pstore_dump may be inconsistently modified
> diff -u -p a/fs/pstore/platform.c b/fs/pstore/platform.c
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -275,7 +275,7 @@ void pstore_record_init(struct pstore_re
> * end of the buffer.
> */
> static void pstore_dump(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> + struct kmsg_dump_detail *detail)
> {
> struct kmsg_dump_iter iter;
> unsigned long total = 0;
> @@ -285,9 +285,9 @@ static void pstore_dump(struct kmsg_dump
> int saved_ret = 0;
> int ret;
>
> - why = kmsg_dump_reason_str(reason);
> + why = kmsg_dump_reason_str(detail->reason);
>
> - if (pstore_cannot_block_path(reason)) {
> + if (pstore_cannot_block_path(detail->reason)) {
> if (!spin_trylock_irqsave(&psinfo->buf_lock, flags)) {
> pr_err("dump skipped in %s path because of concurrent dump\n",
> in_nmi() ? "NMI" : why);
> @@ -311,7 +311,7 @@ static void pstore_dump(struct kmsg_dump
> pstore_record_init(&record, psinfo);
> record.type = PSTORE_TYPE_DMESG;
> record.count = oopscount;
> - record.reason = reason;
> + record.reason = detail->reason;
> record.part = part;
> record.buf = psinfo->buf;
>
> @@ -352,7 +352,7 @@ static void pstore_dump(struct kmsg_dump
> }
>
> ret = psinfo->write(&record);
> - if (ret == 0 && reason == KMSG_DUMP_OOPS) {
> + if (ret == 0 && detail->reason == KMSG_DUMP_OOPS) {
> pstore_new_entry = 1;
> pstore_timer_kick();
> } else {
> diff -u -p a/arch/um/kernel/kmsg_dump.c b/arch/um/kernel/kmsg_dump.c
> --- a/arch/um/kernel/kmsg_dump.c
> +++ b/arch/um/kernel/kmsg_dump.c
> @@ -8,7 +8,7 @@
> #include <os.h>
>
> static void kmsg_dumper_stdout(struct kmsg_dumper *dumper,
> - enum kmsg_dump_reason reason)
> + struct kmsg_dump_detail *detail)
> {
> static struct kmsg_dump_iter iter;
> static DEFINE_SPINLOCK(lock);
>
^ permalink raw reply
* Re: [PATCH v3] net: mana: Fix possible double free in error handling path
From: patchwork-bot+netdevbpf @ 2024-06-27 10:40 UTC (permalink / raw)
To: Ma Ke
Cc: kys, haiyangz, wei.liu, decui, davem, edumazet, kuba, pabeni,
shradhagupta, horms, kotaranov, longli, schakrabarti,
erick.archer, leon, linux-hyperv, netdev, linux-kernel
In-Reply-To: <20240625130314.2661257-1-make24@iscas.ac.cn>
Hello:
This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:
On Tue, 25 Jun 2024 21:03:14 +0800 you wrote:
> When auxiliary_device_add() returns error and then calls
> auxiliary_device_uninit(), callback function adev_release
> calls kfree(madev). We shouldn't call kfree(madev) again
> in the error handling path. Set 'madev' to NULL.
>
> Fixes: a69839d4327d ("net: mana: Add support for auxiliary device")
> Signed-off-by: Ma Ke <make24@iscas.ac.cn>
>
> [...]
Here is the summary with links:
- [v3] net: mana: Fix possible double free in error handling path
https://git.kernel.org/netdev/net/c/1864b8224195
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox