[PATCH v3] kthread_worker: Prevent queuing delayed work from timer_fn when it is being canceled
From: Zqiang There is a small race window when a delayed work is being canceled and the work still might be queued from the timer_fn: CPU0CPU1 kthread_cancel_delayed_work_sync() __kthread_cancel_work_sync() __kthread_cancel_work() work->canceling++; kthread_delayed_work_timer_fn() kthread_insert_work(); BUG: kthread_insert_work() should not get called when work->canceling is set. Reviewed-by: Petr Mladek Signed-off-by: Zqiang --- v1->v2->v3: Change the description of the problem and add 'Reviewed-by' tags. kernel/kthread.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/kthread.c b/kernel/kthread.c index 3edaa380dc7b..85a2c9b32049 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -897,7 +897,8 @@ void kthread_delayed_work_timer_fn(struct timer_list *t) /* Move the work from worker->delayed_work_list. */ WARN_ON_ONCE(list_empty(>node)); list_del_init(>node); - kthread_insert_work(worker, work, >work_list); + if (!work->canceling) + kthread_insert_work(worker, work, >work_list); raw_spin_unlock_irqrestore(>lock, flags); } -- 2.17.1
Re: [PATCH 6/6] USB: cdc-acm: blacklist ETAS ES58X device
On Sun, Sep 27, 2020 at 07:45:20AM +0200, Greg Kroah-Hartman wrote: > On Sun, Sep 27, 2020 at 02:57:56AM +0900, Vincent Mailhol wrote: > > The ES58X devices are incorrectly recognized as USB Modem (CDC ACM), > > preventing the etas-es58x module to load. > > > > Thus, these have been added > > to the ignore list in drivers/usb/class/cdc-acm.c > > > > Signed-off-by: Vincent Mailhol > > --- > > drivers/usb/class/cdc-acm.c | 11 +++ > > 1 file changed, 11 insertions(+) > > Did you mean to send this twice? > > And where are the 5 other patches in this series? > > And finally, it's a good idea to include the output of 'lsusb -v' for > devices that need quirks so we can figure things out later on, can you > fix up your changelog to include that information? Also, why is the device saying it is a cdc-acm compliant device when it is not? Why lie to the operating system like that? thanks, greg k-h
Re: [PATCH v3] RISC-V: Check clint_time_val before use
On Sun, 2020-09-27 at 11:09 +0530, Anup Patel wrote:
> The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> because clint_time_val is used even before CLINT driver is probed
> at following places:
> 1. rand_initialize() calls get_cycles() which in-turn uses
>clint_time_val
> 2. boot_init_stack_canary() calls get_cycles() which in-turn
>uses clint_time_val
>
> The issue#1 (above) is fixed by providing custom random_get_entropy()
> for RISC-V NoMMU kernel. For issue#2 (above), we remove dependency of
> boot_init_stack_canary() on get_cycles() and this is aligned with the
> boot_init_stack_canary() implementations of ARM, ARM64 and MIPS kernel.
>
> Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> for M-mode systems")
> Signed-off-by: Palmer Dabbelt
> Signed-off-by: Anup Patel
> ---
> Changes since v2:
> - Take different approach and provide custom random_get_entropy() for
>RISC-V NoMMU kernel
> - Remove dependency of boot_init_stack_canary() on get_cycles()
> - Hopefully we don't require to set clint_time_val = NULL in CLINT
>driver with a different approach to fix.
> Changes since v1:
> - Explicitly initialize clint_time_val to NULL in CLINT driver to
>avoid hang on Kendryte K210
> ---
> arch/riscv/include/asm/stackprotector.h | 4
> arch/riscv/include/asm/timex.h | 13 +
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/include/asm/stackprotector.h
> b/arch/riscv/include/asm/stackprotector.h
> index d95f7b2a7f37..5962f8891f06 100644
> --- a/arch/riscv/include/asm/stackprotector.h
> +++ b/arch/riscv/include/asm/stackprotector.h
> @@ -5,7 +5,6 @@
>
> #include
> #include
> -#include
>
> extern unsigned long __stack_chk_guard;
>
> @@ -18,12 +17,9 @@ extern unsigned long __stack_chk_guard;
> static __always_inline void boot_init_stack_canary(void)
> {
> unsigned long canary;
> - unsigned long tsc;
>
> /* Try to get a semi random initial value. */
> get_random_bytes(, sizeof(canary));
> - tsc = get_cycles();
> - canary += tsc + (tsc << BITS_PER_LONG/2);
> canary ^= LINUX_VERSION_CODE;
> canary &= CANARY_MASK;
>
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index 7f659dda0032..ab104905d4db 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -33,6 +33,19 @@ static inline u32 get_cycles_hi(void)
> #define get_cycles_hi get_cycles_hi
> #endif /* CONFIG_64BIT */
>
> +/*
> + * Much like MIPS, we may not have a viable counter to use at an early point
> + * in the boot process. Unfortunately we don't have a fallback, so instead
> + * we just return 0.
> + */
> +static inline unsigned long random_get_entropy(void)
> +{
> + if (unlikely(clint_time_val == NULL))
> + return 0;
> + return get_cycles();
> +}
> +#define random_get_entropy() random_get_entropy()
> +
> #else /* CONFIG_RISCV_M_MODE */
>
> static inline cycles_t get_cycles(void)
Did not reply to the patch... So again for Kendryte:
Tested-by: Damien Le Moal
--
Damien Le Moal
Western Digital
Re: [PATCH v3] RISC-V: Check clint_time_val before use
On Sat, 2020-09-26 at 22:46 -0700, Palmer Dabbelt wrote:
> On Sat, 26 Sep 2020 22:39:16 PDT (-0700), Anup Patel wrote:
> > The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> > because clint_time_val is used even before CLINT driver is probed
> > at following places:
> > 1. rand_initialize() calls get_cycles() which in-turn uses
> >clint_time_val
> > 2. boot_init_stack_canary() calls get_cycles() which in-turn
> >uses clint_time_val
> >
> > The issue#1 (above) is fixed by providing custom random_get_entropy()
> > for RISC-V NoMMU kernel. For issue#2 (above), we remove dependency of
> > boot_init_stack_canary() on get_cycles() and this is aligned with the
> > boot_init_stack_canary() implementations of ARM, ARM64 and MIPS kernel.
> >
> > Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> > for M-mode systems")
> > Signed-off-by: Palmer Dabbelt
> > Signed-off-by: Anup Patel
> > ---
> > Changes since v2:
> > - Take different approach and provide custom random_get_entropy() for
> >RISC-V NoMMU kernel
> > - Remove dependency of boot_init_stack_canary() on get_cycles()
> > - Hopefully we don't require to set clint_time_val = NULL in CLINT
> >driver with a different approach to fix.
> > Changes since v1:
> > - Explicitly initialize clint_time_val to NULL in CLINT driver to
> >avoid hang on Kendryte K210
> > ---
> > arch/riscv/include/asm/stackprotector.h | 4
> > arch/riscv/include/asm/timex.h | 13 +
> > 2 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/stackprotector.h
> > b/arch/riscv/include/asm/stackprotector.h
> > index d95f7b2a7f37..5962f8891f06 100644
> > --- a/arch/riscv/include/asm/stackprotector.h
> > +++ b/arch/riscv/include/asm/stackprotector.h
> > @@ -5,7 +5,6 @@
> >
> > #include
> > #include
> > -#include
> >
> > extern unsigned long __stack_chk_guard;
> >
> > @@ -18,12 +17,9 @@ extern unsigned long __stack_chk_guard;
> > static __always_inline void boot_init_stack_canary(void)
> > {
> > unsigned long canary;
> > - unsigned long tsc;
> >
> > /* Try to get a semi random initial value. */
> > get_random_bytes(, sizeof(canary));
> > - tsc = get_cycles();
> > - canary += tsc + (tsc << BITS_PER_LONG/2);
> > canary ^= LINUX_VERSION_CODE;
> > canary &= CANARY_MASK;
> >
> > diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> > index 7f659dda0032..ab104905d4db 100644
> > --- a/arch/riscv/include/asm/timex.h
> > +++ b/arch/riscv/include/asm/timex.h
> > @@ -33,6 +33,19 @@ static inline u32 get_cycles_hi(void)
> > #define get_cycles_hi get_cycles_hi
> > #endif /* CONFIG_64BIT */
> >
> > +/*
> > + * Much like MIPS, we may not have a viable counter to use at an early
> > point
> > + * in the boot process. Unfortunately we don't have a fallback, so instead
> > + * we just return 0.
> > + */
> > +static inline unsigned long random_get_entropy(void)
> > +{
> > + if (unlikely(clint_time_val == NULL))
> > + return 0;
> > + return get_cycles();
> > +}
> > +#define random_get_entropy() random_get_entropy()
> > +
> > #else /* CONFIG_RISCV_M_MODE */
> >
> > static inline cycles_t get_cycles(void)
>
> Reviewed-by: Palmer Dabbelt
>
> I'm going to wait for Damien to chime in about the NULL initialization boot
> failure, though, as I'm a bit worried something else was going on.
>
> Thanks!
For Kendryte, no problems. Boots correctly.
Tested-by: Damien Le Moal
--
Damien Le Moal
Western Digital
Re: [PATCH v3 1/5] fpga: dfl: rename the bus type "dfl" to "fpga-dfl"
On Sat, Sep 26, 2020 at 12:22:19PM -0700, Moritz Fischer wrote: > Hi Greg, > > On Sat, Sep 26, 2020 at 08:09:13AM +0200, Greg KH wrote: > > On Sat, Sep 26, 2020 at 10:23:46AM +0800, Xu Yilun wrote: > > > Hi greg, > > > > > > About the bus naming, I summarized some questions we've discussed to check > > > with you. See inline. > > > > > > On Thu, Sep 24, 2020 at 10:27:00AM -0700, Moritz Fischer wrote: > > > > Hi Xu, > > > > > > > > On Fri, Sep 25, 2020 at 12:59:57AM +0800, Xu Yilun wrote: > > > > > Now the DFL device drivers could be made as independent modules and > > > > > put > > > > > in different subsystems according to their functionalities. So the > > > > > name > > > > > should be descriptive and unique in the whole kernel. > > > > > > > > > > The patch changes the naming of dfl bus related structures, functions, > > > > > APIs and documentations. > > > > > > > > > > Signed-off-by: Xu Yilun > > > > > --- > > > > > Documentation/ABI/testing/sysfs-bus-dfl | 15 -- > > > > > Documentation/ABI/testing/sysfs-bus-fpga-dfl | 15 ++ > > > > > MAINTAINERS | 2 +- > > > > > drivers/fpga/dfl.c | 254 > > > > > ++- > > > > > drivers/fpga/dfl.h | 77 > > > > > 5 files changed, 184 insertions(+), 179 deletions(-) > > > > > delete mode 100644 Documentation/ABI/testing/sysfs-bus-dfl > > > > > create mode 100644 Documentation/ABI/testing/sysfs-bus-fpga-dfl > > > > > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-dfl > > > > > b/Documentation/ABI/testing/sysfs-bus-dfl > > > > > deleted file mode 100644 > > > > > index 23543be..000 > > > > > --- a/Documentation/ABI/testing/sysfs-bus-dfl > > > > > +++ /dev/null > > > > > @@ -1,15 +0,0 @@ > > > > > -What:/sys/bus/dfl/devices/dfl_dev.X/type > > > > > -Date:Aug 2020 > > > > > -KernelVersion: 5.10 > > > > > -Contact: Xu Yilun > > > > > -Description: Read-only. It returns type of DFL FIU of the device. > > > > > Now DFL > > > > > - supports 2 FIU types, 0 for FME, 1 for PORT. > > > > > - Format: 0x%x > > > > > - > > > > > -What:/sys/bus/dfl/devices/dfl_dev.X/feature_id > > > > > -Date:Aug 2020 > > > > > -KernelVersion: 5.10 > > > > > -Contact: Xu Yilun > > > > > -Description: Read-only. It returns feature identifier local to its > > > > > DFL FIU > > > > > - type. > > > > > - Format: 0x%x > > > > > > > > You're changing userland facing ABI. I think that's something to avoid, > > > > please check with Greg on the rules since this hasn't been in a release > > > > yet. > > > > > > > > > > I'm going to change the name of bus stuff for other subsystems, to be > > > aligned, I also consider change the bus_type.name and dfl dev_name. But > > > it will cause the changing of user ABIs. No user case for these user ABI > > > now cause they are just queued, is it good I change them? > > > > Why change the user name here? No need for that, right? Unless you > > really want to, and think that no one will notice. If so, fine, change > > them :) > > Let's leave it as is -- An FPGA is one possible implementation and as for > other buses, you wouldn't call it fpga-usb or usb-fpga just because the > USB bus is implemented in an FPGA if it behaves like a normal USB bus. > Having an ASIC based DFL bus show up under dfl-fpga / fpga-dfl in sysfs > would be super confusing. > > > > It is mentioned that although Device Feature List is introduced in FPGA, > > > but it doesn't limit the usage in FPGA only. It's just a method to > > > discover features from a device, for sure it can be extended and used > > > in other devices too. So it can be bigger namespace than FPGA. Like in > > > our existing code, we picked dfl_fpga (DFL based FPGA) for uapi (ioctl) > > > and internal functions. This is suggested by Alan (The previous FPGA > > > maintainer). It's possible to have "DFL based XXX" in the future, even > > > currently only FPGA uses DFL. This is the reason we thought just "dfl" > > > in the whole kernel space is OK. > > > So, is there a chance we keep the "dfl" naming in the whole kernel? > > > > No one knows what "DFL" is, and odds are, if a different subsystem wants > > to use it, they will have their own variant, right? > > > > And why didn't you all use device tree? How did this sneak in past > > everyone? > > DFL is a pretty efficient implementation in terms of resource > utilization on the FPGA end (a couple of registers / memories) vs > several kilobytes of memory for a device-tree blob. > > The hardware using DFL to describe its internal structure exists in the > form of deployed accelerator cards and telling all its users to go and > change their hardware design would be feasible -- If you think about an > FPGA as a (albeit reconfigurable) ASIC you wouldn't go and tell people > to
Re: [PATCH] Bluetooth: Fix the vulnerable issue on enc key size
Hi Marcel,
> On 26 September 2020 at 1:34, Marcel Holtmann wrote:
>
> Hi Alex,
>
> >>> When someone attacks the service provider, it creates connection,
> >>> authenticates. Then it requests key size of one byte and it identifies
> >>> the key with brute force methods.
> >>>
> >>> After l2cap info req/resp exchange is complete. the attacker sends l2cap
> >>> connect with specific PSM.
> >>>
> >>> In above procedure, there is no chance for the service provider to check
> >>> the encryption key size before l2cap_connect(). Because the state of
> >>> l2cap chan in conn->chan_l is BT_LISTEN, there is no l2cap chan with the
> >>> state of BT_CONNECT or BT_CONNECT2.
> >>>
> >>> So service provider should check the encryption key size in
> >>> l2cap_connect()
> >>>
> >>> Signed-off-by: Alex Lu
> >>> ---
> >>> net/bluetooth/l2cap_core.c | 7 +++
> >>> 1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >>> index ade83e224567..63df961d402d 100644
> >>> --- a/net/bluetooth/l2cap_core.c
> >>> +++ b/net/bluetooth/l2cap_core.c
> >>> @@ -4150,6 +4150,13 @@ static struct l2cap_chan *l2cap_connect(struct
> >> l2cap_conn *conn,
> >>>
> >>> if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
> >>> if (l2cap_chan_check_security(chan, false)) {
> >>> + if (!l2cap_check_enc_key_size(conn->hcon)) {
> >>> + l2cap_state_change(chan, BT_DISCONN);
> >>> + __set_chan_timer(chan,
> >> L2CAP_DISC_TIMEOUT);
> >>> + result = L2CAP_CR_SEC_BLOCK;
> >>> + status = L2CAP_CS_NO_INFO;
> >>> + goto response;
> >>> + }
> >>> if (test_bit(FLAG_DEFER_SETUP, >flags)) {
> >>> l2cap_state_change(chan, BT_CONNECT2);
> >>> result = L2CAP_CR_PEND;
> >>
> >> I am not following what you are trying to fix here. Can you show this with
> a
> >> btmon trace from an attacking device?
> >>
> >> Regards
> >>
> >> Marcel
> >>
> >>
> >
> > I'm sorry, I didn't have btmon trace from an attacking device.
> > I didn't have the real attacking device. I just simulate the attacking.
> > I have a device that can create one byte size encryption key.
> > It uses the link key that was produced by pairing with the service provider.
> Actually the KNOB (Key Negotiation of Bluetooth Attack) says, the link key is
> unnecessary for the reconnection.
> > I use this device to reconnect to service provider, and then initiate the
> > Key
> Negotiation for one byte size encryption key. Actually the attacker identified
> the encryption key with some brute force methods.
> >
> > I want to provide the trace on service provider side.
>
> what kernel version are you running? I wonder if we should always return
> L2CAP_CR_PEND here. Do you have a reproducer code?
I'm running kernel 5.8.0-rc6 on acceptor and kernel 5.8.5 on the initiator
which acts as an attacker.
For the attack simulation, some code needs to be changed on each size.
On the acceptor, the master parameter for bt_io_listen() in bluetoothd should
be changed to FALSE in profiles/audio/a2dp.c a2dp_server_listen() and
profiles/audio/avctp.c avctp_server_socket().
The change makes the kernel not to change the role to master when it receives
hci conn req event.
I did the change in order to make the controller to send
LMP_ENCRYPTION_KEY_SIZE_REQ PDU for one byte key size.
On the initiator, the below encryption key size check should be removed.
@@ -1622,10 +1624,13 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
continue;
}
- if (l2cap_check_enc_key_size(conn->hcon))
- l2cap_start_connection(chan);
- else
- l2cap_chan_close(chan, ECONNREFUSED);
+ /* Just simulate KNOB */
+ l2cap_start_connection(chan);
+ /* if (l2cap_check_enc_key_size(conn->hcon))
+* l2cap_start_connection(chan);
+* else
+* l2cap_chan_close(chan, ECONNREFUSED);
+*/
At last, I did the test as below:
1. On the initiator, pair acceptor
2. Run l2test -r -P 3 on the acceptor
3. Run l2test -n -P 3 on the initiator
>
> The problem really is that the MASK_REQ_DONE indication is not enough to
> make a decision for the key size. We have to ensure that also the key size is
> actually available. If that is not yet done, then we should not check it. This
> means that any response to L2CAP_Connect_Request PDU needs to be
> delayed until the key size has been read.
In my test case, the key size has been read from controller before the l2cap
conn request PDU is received.
< HCI Command: Read Encryption Key Size (0x05|0x0008)
Re: [PATCH v3] RISC-V: Check clint_time_val before use
On Sat, 26 Sep 2020 22:39:16 PDT (-0700), Anup Patel wrote:
The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
because clint_time_val is used even before CLINT driver is probed
at following places:
1. rand_initialize() calls get_cycles() which in-turn uses
clint_time_val
2. boot_init_stack_canary() calls get_cycles() which in-turn
uses clint_time_val
The issue#1 (above) is fixed by providing custom random_get_entropy()
for RISC-V NoMMU kernel. For issue#2 (above), we remove dependency of
boot_init_stack_canary() on get_cycles() and this is aligned with the
boot_init_stack_canary() implementations of ARM, ARM64 and MIPS kernel.
Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
for M-mode systems")
Signed-off-by: Palmer Dabbelt
Signed-off-by: Anup Patel
---
Changes since v2:
- Take different approach and provide custom random_get_entropy() for
RISC-V NoMMU kernel
- Remove dependency of boot_init_stack_canary() on get_cycles()
- Hopefully we don't require to set clint_time_val = NULL in CLINT
driver with a different approach to fix.
Changes since v1:
- Explicitly initialize clint_time_val to NULL in CLINT driver to
avoid hang on Kendryte K210
---
arch/riscv/include/asm/stackprotector.h | 4
arch/riscv/include/asm/timex.h | 13 +
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/include/asm/stackprotector.h
b/arch/riscv/include/asm/stackprotector.h
index d95f7b2a7f37..5962f8891f06 100644
--- a/arch/riscv/include/asm/stackprotector.h
+++ b/arch/riscv/include/asm/stackprotector.h
@@ -5,7 +5,6 @@
#include
#include
-#include
extern unsigned long __stack_chk_guard;
@@ -18,12 +17,9 @@ extern unsigned long __stack_chk_guard;
static __always_inline void boot_init_stack_canary(void)
{
unsigned long canary;
- unsigned long tsc;
/* Try to get a semi random initial value. */
get_random_bytes(, sizeof(canary));
- tsc = get_cycles();
- canary += tsc + (tsc << BITS_PER_LONG/2);
canary ^= LINUX_VERSION_CODE;
canary &= CANARY_MASK;
diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 7f659dda0032..ab104905d4db 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -33,6 +33,19 @@ static inline u32 get_cycles_hi(void)
#define get_cycles_hi get_cycles_hi
#endif /* CONFIG_64BIT */
+/*
+ * Much like MIPS, we may not have a viable counter to use at an early point
+ * in the boot process. Unfortunately we don't have a fallback, so instead
+ * we just return 0.
+ */
+static inline unsigned long random_get_entropy(void)
+{
+ if (unlikely(clint_time_val == NULL))
+ return 0;
+ return get_cycles();
+}
+#define random_get_entropy() random_get_entropy()
+
#else /* CONFIG_RISCV_M_MODE */
static inline cycles_t get_cycles(void)
Reviewed-by: Palmer Dabbelt
I'm going to wait for Damien to chime in about the NULL initialization boot
failure, though, as I'm a bit worried something else was going on.
Thanks!
Re: [PATCH 22/24] membarrier.2: Note that glibc does not provide a wrapper
At 2020-09-24T10:06:23+0200, Michael Kerrisk (man-pages) wrote: > Thanks for the interesting history, Branden! Hi, Michael. And you're welcome! I often wonder if I test people's patience with my info dumps but I try to show my work when making claims. > From time toi time I wonder if the function prototypes in > the SYNOPSIS should also be inside .EX/.EE. Your thoughts? I think there are trade-offs. 1. If you want alignment, the monospaced font that .EX/.EE uses is the most portable way to get it. 2. For typeset output, you'll generally run out of line more quickly with a monospaced font than with the troff/man default (Times). _Any_ time filling is off, output should be checked to see if it overruns the right margin, but this point strengthens in monospace. Here's something that isn't a trade-off that might come as a surprise to some readers. * You can still get bold and italics inside an .EX/.EE region, so you can still use these distinguish data types, variable names, and what-have-you. The idiom for achieving this is apparently not well-known among those who write man pages by hand, and tools that generate man(7) language from some other source often produce output that is so ugly as to be unintelligible to non-experts in *roff. The key insights are that (A) you can still make macro calls inside an .EX/.EE region, and (B) you can use the \c escape to "interrupt" an input line and continue it on the next without introducing any whitespace. For instance, looking at fstat() from your stat(2) page, I might write it using .EX and .EE as follows: .EX .B int fstat(int \c .IB fd , \~\c .B struct stat *\c .IB statbuf ); .EE Normally in man pages, it is senseless to have any spaces before the \c escape, and \c is best omitted in that event. However, when filling is disabled (as is the case in .EX/.EE regions), output lines break where the input lines do by default--\c overrides this, causing the lines to be joined. (Regarding the \~, see below.) If there is no use for roman in the line, then you could do the whole function signature with the .BI macro by quoting macro arguments that contain whitespace. .EX .BI "int fstat(int " fd ", struct stat *" statbuf ); .EE As a matter of personal style, I find quoted space characters interior but adjacent to quotation marks visually confusing--it's slower for me to tell which parts of the line are "inside" the quotes and which outside--so I turn to groff's \~ non-breaking space escape (widely supported elsewhere) for these boundary spaces. .EX .BI "int fstat(int\~" fd ", struct stat *" statbuf ); .EE Which of the above three models do you think would work best for the man-pages project? Also, do you have any use for roman in function signatures? I see it used for comments and feature test macro material, but not within function signatures proper. As an aside, I will admit to some unease with the heavy use of bold in synopses in section 2 and 3 man pages, but I can marshal no historical argument against it. In fact, a quick check of some Unix v7 section 2 pages from 1979 that I have lying around (thanks to TUHS) reveals that Bell Labs used undifferentiated bold for the whole synopsis! $ head -n 13 usr/man/man2/stat.2 .TH STAT 2 .SH NAME stat, fstat \- get file status .SH SYNOPSIS .B #include .br .B #include .PP .B stat(name, buf) .br .B char *name; .br .B struct stat *buf; Regards, Branden signature.asc Description: PGP signature
Re: [PATCH 6/6] USB: cdc-acm: blacklist ETAS ES58X device
On Sun, Sep 27, 2020 at 02:57:56AM +0900, Vincent Mailhol wrote: > The ES58X devices are incorrectly recognized as USB Modem (CDC ACM), > preventing the etas-es58x module to load. > > Thus, these have been added > to the ignore list in drivers/usb/class/cdc-acm.c > > Signed-off-by: Vincent Mailhol > --- > drivers/usb/class/cdc-acm.c | 11 +++ > 1 file changed, 11 insertions(+) Did you mean to send this twice? And where are the 5 other patches in this series? And finally, it's a good idea to include the output of 'lsusb -v' for devices that need quirks so we can figure things out later on, can you fix up your changelog to include that information? thanks, greg k-h
Re: [PATCH v4 00/17] HSM driver for ACRN hypervisor
On Sun, Sep 27, 2020 at 08:24:39AM +0800, Liu, Shuo A wrote: > Ping... It's been less than a week since you sent this. Please relax and if you really need reviews, get them from within Intel, where you can impose a deadline on those developers. Otherwise, your patch is in good company: $ mdfrm -c ~/mail/todo/ 993 messages in /home/gregkh/mail/todo/ And will be handled when I get to it. thanks, greg "Intel still owes me lots of liquor" k-h
[PATCH v3] RISC-V: Check clint_time_val before use
The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
because clint_time_val is used even before CLINT driver is probed
at following places:
1. rand_initialize() calls get_cycles() which in-turn uses
clint_time_val
2. boot_init_stack_canary() calls get_cycles() which in-turn
uses clint_time_val
The issue#1 (above) is fixed by providing custom random_get_entropy()
for RISC-V NoMMU kernel. For issue#2 (above), we remove dependency of
boot_init_stack_canary() on get_cycles() and this is aligned with the
boot_init_stack_canary() implementations of ARM, ARM64 and MIPS kernel.
Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
for M-mode systems")
Signed-off-by: Palmer Dabbelt
Signed-off-by: Anup Patel
---
Changes since v2:
- Take different approach and provide custom random_get_entropy() for
RISC-V NoMMU kernel
- Remove dependency of boot_init_stack_canary() on get_cycles()
- Hopefully we don't require to set clint_time_val = NULL in CLINT
driver with a different approach to fix.
Changes since v1:
- Explicitly initialize clint_time_val to NULL in CLINT driver to
avoid hang on Kendryte K210
---
arch/riscv/include/asm/stackprotector.h | 4
arch/riscv/include/asm/timex.h | 13 +
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/include/asm/stackprotector.h
b/arch/riscv/include/asm/stackprotector.h
index d95f7b2a7f37..5962f8891f06 100644
--- a/arch/riscv/include/asm/stackprotector.h
+++ b/arch/riscv/include/asm/stackprotector.h
@@ -5,7 +5,6 @@
#include
#include
-#include
extern unsigned long __stack_chk_guard;
@@ -18,12 +17,9 @@ extern unsigned long __stack_chk_guard;
static __always_inline void boot_init_stack_canary(void)
{
unsigned long canary;
- unsigned long tsc;
/* Try to get a semi random initial value. */
get_random_bytes(, sizeof(canary));
- tsc = get_cycles();
- canary += tsc + (tsc << BITS_PER_LONG/2);
canary ^= LINUX_VERSION_CODE;
canary &= CANARY_MASK;
diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 7f659dda0032..ab104905d4db 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -33,6 +33,19 @@ static inline u32 get_cycles_hi(void)
#define get_cycles_hi get_cycles_hi
#endif /* CONFIG_64BIT */
+/*
+ * Much like MIPS, we may not have a viable counter to use at an early point
+ * in the boot process. Unfortunately we don't have a fallback, so instead
+ * we just return 0.
+ */
+static inline unsigned long random_get_entropy(void)
+{
+ if (unlikely(clint_time_val == NULL))
+ return 0;
+ return get_cycles();
+}
+#define random_get_entropy() random_get_entropy()
+
#else /* CONFIG_RISCV_M_MODE */
static inline cycles_t get_cycles(void)
--
2.25.1
Re: [PATCH v2] RISC-V: Check clint_time_val before use
On Sat, 26 Sep 2020 22:38:17 PDT (-0700), a...@brainfault.org wrote:
On Sun, Sep 27, 2020 at 5:50 AM Palmer Dabbelt wrote:
On Sat, 26 Sep 2020 03:31:29 PDT (-0700), Damien Le Moal wrote:
> On Sat, 2020-09-26 at 15:51 +0530, Anup Patel wrote:
>> The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
>> because the get_cycles() and friends are called very early from
>> rand_initialize() before CLINT driver is probed. To fix this, we
>> should check clint_time_val before use in get_cycles() and friends.
I don't think this is the right way to solve that problem, as we're essentially
just lying about the timer rather than informing the system we can't get
timer-based entropy right now. MIPS is explicit about this, I don't see any
reason why we shouldn't be as well.
Does this fix the boot issue (see below for the NULL)? There's some other
random-related arch functions so this might not be quite the right way to do
it.
diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 7f659dda0032..7e39b0068932 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -33,6 +33,18 @@ static inline u32 get_cycles_hi(void)
#define get_cycles_hi get_cycles_hi
#endif /* CONFIG_64BIT */
+/*
+ * Much like MIPS, we may not have a viable counter to use at an early point in
+ * the boot process. Unfortunately we don't have a fallback, so instead we
+ * just return 0.
+ */
+static inline unsigned long random_get_entropy(void)
+{
+ if (unlikely(clint_time_val == NULL))
+ return 0;
+ return get_cycles();
+}
+
#else /* CONFIG_RISCV_M_MODE */
static inline cycles_t get_cycles(void)
>> Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
>> for M-mode systems")
>> Signed-off-by: Anup Patel
>> ---
>> Changes since v1:
>> - Explicitly initialize clint_time_val to NULL in CLINT driver to
>>avoid hang on Kendryte K210
>> ---
>> arch/riscv/include/asm/timex.h| 12 +---
>> drivers/clocksource/timer-clint.c | 2 +-
>> 2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
>> index 7f659dda0032..6e7b04874755 100644
>> --- a/arch/riscv/include/asm/timex.h
>> +++ b/arch/riscv/include/asm/timex.h
>> @@ -17,18 +17,24 @@ typedef unsigned long cycles_t;
>> #ifdef CONFIG_64BIT
>> static inline cycles_t get_cycles(void)
>> {
>> -return readq_relaxed(clint_time_val);
>> +if (clint_time_val)
>> +return readq_relaxed(clint_time_val);
>> +return 0;
>> }
>> #else /* !CONFIG_64BIT */
>> static inline u32 get_cycles(void)
>> {
>> -return readl_relaxed(((u32 *)clint_time_val));
>> +if (clint_time_val)
>> +return readl_relaxed(((u32 *)clint_time_val));
>> +return 0;
>> }
>> #define get_cycles get_cycles
>>
>> static inline u32 get_cycles_hi(void)
>> {
>> -return readl_relaxed(((u32 *)clint_time_val) + 1);
>> +if (clint_time_val)
>> +return readl_relaxed(((u32 *)clint_time_val) + 1);
>> +return 0;
>> }
>> #define get_cycles_hi get_cycles_hi
>> #endif /* CONFIG_64BIT */
>> diff --git a/drivers/clocksource/timer-clint.c
b/drivers/clocksource/timer-clint.c
>> index d17367dee02c..8dbec85979fd 100644
>> --- a/drivers/clocksource/timer-clint.c
>> +++ b/drivers/clocksource/timer-clint.c
>> @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
>> static unsigned int clint_timer_irq;
>>
>> #ifdef CONFIG_RISCV_M_MODE
>> -u64 __iomem *clint_time_val;
>> +u64 __iomem *clint_time_val = NULL;
This one I definately don't get. According the internet, the C standard says
If an object that has static storage duration is not initialized
explicitly, it is initialized implicitly as if every member that has
arithmetic type were assigned 0 and every member that has pointer type were
assigned a null pointer constant.
so unless I'm missing something there shouldn't be any difference between these
two lines. When I just apply this I get exactly the same "objdump -dt" before
and after. I do see some difference in assembly, but only when I don't pass
"-fno-common" and that ends up being passed during my Linux builds.
Even checkpatch complains about explicit NULL assignment to global variables.
I will send v3 with a different approach to fix this so hopefully with
v3 we will
not require explicit NULL assignment.
Awesome, thanks!
Regards,
Anup
>> #endif
>>
>> static void clint_send_ipi(const struct cpumask *target)
>
> For Kendryte:
>
> Tested-by: Damien Le Moal
>
> --
> Damien Le Moal
> Western Digital
Re: [PATCH v2] RISC-V: Check clint_time_val before use
On Sat, 26 Sep 2020 22:35:39 PDT (-0700), a...@brainfault.org wrote:
On Sun, Sep 27, 2020 at 5:50 AM Palmer Dabbelt wrote:
On Sat, 26 Sep 2020 03:31:29 PDT (-0700), Damien Le Moal wrote:
> On Sat, 2020-09-26 at 15:51 +0530, Anup Patel wrote:
>> The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
>> because the get_cycles() and friends are called very early from
>> rand_initialize() before CLINT driver is probed. To fix this, we
>> should check clint_time_val before use in get_cycles() and friends.
I don't think this is the right way to solve that problem, as we're essentially
just lying about the timer rather than informing the system we can't get
timer-based entropy right now. MIPS is explicit about this, I don't see any
reason why we shouldn't be as well.
Does this fix the boot issue (see below for the NULL)? There's some other
random-related arch functions so this might not be quite the right way to do
it.
diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 7f659dda0032..7e39b0068932 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -33,6 +33,18 @@ static inline u32 get_cycles_hi(void)
#define get_cycles_hi get_cycles_hi
#endif /* CONFIG_64BIT */
+/*
+ * Much like MIPS, we may not have a viable counter to use at an early point in
+ * the boot process. Unfortunately we don't have a fallback, so instead we
+ * just return 0.
+ */
+static inline unsigned long random_get_entropy(void)
+{
+ if (unlikely(clint_time_val == NULL))
+ return 0;
+ return get_cycles();
+}
+
Overall, this approach is good but this change is incomplete so does not work.
The linux/timex.h expects random_get_entropy() to be macro so we need a
"#define" as well.
After fixing rand_initialize() with custom random_get_entropy(), we get another
issue in boot_init_stack_canary() because boot_init_stack_canary() directly
calls get_cycles() so we remove use of get_cycles() from
boot_init_stack_canary()
and this is similar to ARM, ARM64, and MIPS kernel.
OK, are you going to send that patch or do you want me to?
Regards,
Anup
#else /* CONFIG_RISCV_M_MODE */
static inline cycles_t get_cycles(void)
>> Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
>> for M-mode systems")
>> Signed-off-by: Anup Patel
>> ---
>> Changes since v1:
>> - Explicitly initialize clint_time_val to NULL in CLINT driver to
>>avoid hang on Kendryte K210
>> ---
>> arch/riscv/include/asm/timex.h| 12 +---
>> drivers/clocksource/timer-clint.c | 2 +-
>> 2 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
>> index 7f659dda0032..6e7b04874755 100644
>> --- a/arch/riscv/include/asm/timex.h
>> +++ b/arch/riscv/include/asm/timex.h
>> @@ -17,18 +17,24 @@ typedef unsigned long cycles_t;
>> #ifdef CONFIG_64BIT
>> static inline cycles_t get_cycles(void)
>> {
>> -return readq_relaxed(clint_time_val);
>> +if (clint_time_val)
>> +return readq_relaxed(clint_time_val);
>> +return 0;
>> }
>> #else /* !CONFIG_64BIT */
>> static inline u32 get_cycles(void)
>> {
>> -return readl_relaxed(((u32 *)clint_time_val));
>> +if (clint_time_val)
>> +return readl_relaxed(((u32 *)clint_time_val));
>> +return 0;
>> }
>> #define get_cycles get_cycles
>>
>> static inline u32 get_cycles_hi(void)
>> {
>> -return readl_relaxed(((u32 *)clint_time_val) + 1);
>> +if (clint_time_val)
>> +return readl_relaxed(((u32 *)clint_time_val) + 1);
>> +return 0;
>> }
>> #define get_cycles_hi get_cycles_hi
>> #endif /* CONFIG_64BIT */
>> diff --git a/drivers/clocksource/timer-clint.c
b/drivers/clocksource/timer-clint.c
>> index d17367dee02c..8dbec85979fd 100644
>> --- a/drivers/clocksource/timer-clint.c
>> +++ b/drivers/clocksource/timer-clint.c
>> @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
>> static unsigned int clint_timer_irq;
>>
>> #ifdef CONFIG_RISCV_M_MODE
>> -u64 __iomem *clint_time_val;
>> +u64 __iomem *clint_time_val = NULL;
This one I definately don't get. According the internet, the C standard says
If an object that has static storage duration is not initialized
explicitly, it is initialized implicitly as if every member that has
arithmetic type were assigned 0 and every member that has pointer type were
assigned a null pointer constant.
so unless I'm missing something there shouldn't be any difference between these
two lines. When I just apply this I get exactly the same "objdump -dt" before
and after. I do see some difference in assembly, but only when I don't pass
"-fno-common" and that ends up being passed during my Linux builds.
>> #endif
>>
>> static void clint_send_ipi(const struct cpumask *target)
>
> For Kendryte:
>
> Tested-by: Damien Le Moal
>
> --
> Damien Le Moal
> Western Digital
Re: [PATCH v2] RISC-V: Check clint_time_val before use
On Sun, Sep 27, 2020 at 5:50 AM Palmer Dabbelt wrote:
>
> On Sat, 26 Sep 2020 03:31:29 PDT (-0700), Damien Le Moal wrote:
> > On Sat, 2020-09-26 at 15:51 +0530, Anup Patel wrote:
> >> The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> >> because the get_cycles() and friends are called very early from
> >> rand_initialize() before CLINT driver is probed. To fix this, we
> >> should check clint_time_val before use in get_cycles() and friends.
>
> I don't think this is the right way to solve that problem, as we're
> essentially
> just lying about the timer rather than informing the system we can't get
> timer-based entropy right now. MIPS is explicit about this, I don't see any
> reason why we shouldn't be as well.
>
> Does this fix the boot issue (see below for the NULL)? There's some other
> random-related arch functions so this might not be quite the right way to do
> it.
>
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index 7f659dda0032..7e39b0068932 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -33,6 +33,18 @@ static inline u32 get_cycles_hi(void)
> #define get_cycles_hi get_cycles_hi
> #endif /* CONFIG_64BIT */
>
> +/*
> + * Much like MIPS, we may not have a viable counter to use at an early point
> in
> + * the boot process. Unfortunately we don't have a fallback, so instead we
> + * just return 0.
> + */
> +static inline unsigned long random_get_entropy(void)
> +{
> + if (unlikely(clint_time_val == NULL))
> + return 0;
> + return get_cycles();
> +}
> +
> #else /* CONFIG_RISCV_M_MODE */
>
> static inline cycles_t get_cycles(void)
>
> >> Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> >> for M-mode systems")
> >> Signed-off-by: Anup Patel
> >> ---
> >> Changes since v1:
> >> - Explicitly initialize clint_time_val to NULL in CLINT driver to
> >>avoid hang on Kendryte K210
> >> ---
> >> arch/riscv/include/asm/timex.h| 12 +---
> >> drivers/clocksource/timer-clint.c | 2 +-
> >> 2 files changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/riscv/include/asm/timex.h
> >> b/arch/riscv/include/asm/timex.h
> >> index 7f659dda0032..6e7b04874755 100644
> >> --- a/arch/riscv/include/asm/timex.h
> >> +++ b/arch/riscv/include/asm/timex.h
> >> @@ -17,18 +17,24 @@ typedef unsigned long cycles_t;
> >> #ifdef CONFIG_64BIT
> >> static inline cycles_t get_cycles(void)
> >> {
> >> -return readq_relaxed(clint_time_val);
> >> +if (clint_time_val)
> >> +return readq_relaxed(clint_time_val);
> >> +return 0;
> >> }
> >> #else /* !CONFIG_64BIT */
> >> static inline u32 get_cycles(void)
> >> {
> >> -return readl_relaxed(((u32 *)clint_time_val));
> >> +if (clint_time_val)
> >> +return readl_relaxed(((u32 *)clint_time_val));
> >> +return 0;
> >> }
> >> #define get_cycles get_cycles
> >>
> >> static inline u32 get_cycles_hi(void)
> >> {
> >> -return readl_relaxed(((u32 *)clint_time_val) + 1);
> >> +if (clint_time_val)
> >> +return readl_relaxed(((u32 *)clint_time_val) + 1);
> >> +return 0;
> >> }
> >> #define get_cycles_hi get_cycles_hi
> >> #endif /* CONFIG_64BIT */
> >> diff --git a/drivers/clocksource/timer-clint.c
> >> b/drivers/clocksource/timer-clint.c
> >> index d17367dee02c..8dbec85979fd 100644
> >> --- a/drivers/clocksource/timer-clint.c
> >> +++ b/drivers/clocksource/timer-clint.c
> >> @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
> >> static unsigned int clint_timer_irq;
> >>
> >> #ifdef CONFIG_RISCV_M_MODE
> >> -u64 __iomem *clint_time_val;
> >> +u64 __iomem *clint_time_val = NULL;
>
> This one I definately don't get. According the internet, the C standard says
>
> If an object that has static storage duration is not initialized
> explicitly, it is initialized implicitly as if every member that has
> arithmetic type were assigned 0 and every member that has pointer type
> were
> assigned a null pointer constant.
>
> so unless I'm missing something there shouldn't be any difference between
> these
> two lines. When I just apply this I get exactly the same "objdump -dt" before
> and after. I do see some difference in assembly, but only when I don't pass
> "-fno-common" and that ends up being passed during my Linux builds.
Even checkpatch complains about explicit NULL assignment to global variables.
I will send v3 with a different approach to fix this so hopefully with
v3 we will
not require explicit NULL assignment.
Regards,
Anup
>
> >> #endif
> >>
> >> static void clint_send_ipi(const struct cpumask *target)
> >
> > For Kendryte:
> >
> > Tested-by: Damien Le Moal
> >
> > --
> > Damien Le Moal
> > Western Digital
Re: [PATCH v2] RISC-V: Check clint_time_val before use
On Sun, Sep 27, 2020 at 5:50 AM Palmer Dabbelt wrote:
>
> On Sat, 26 Sep 2020 03:31:29 PDT (-0700), Damien Le Moal wrote:
> > On Sat, 2020-09-26 at 15:51 +0530, Anup Patel wrote:
> >> The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
> >> because the get_cycles() and friends are called very early from
> >> rand_initialize() before CLINT driver is probed. To fix this, we
> >> should check clint_time_val before use in get_cycles() and friends.
>
> I don't think this is the right way to solve that problem, as we're
> essentially
> just lying about the timer rather than informing the system we can't get
> timer-based entropy right now. MIPS is explicit about this, I don't see any
> reason why we shouldn't be as well.
>
> Does this fix the boot issue (see below for the NULL)? There's some other
> random-related arch functions so this might not be quite the right way to do
> it.
>
> diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
> index 7f659dda0032..7e39b0068932 100644
> --- a/arch/riscv/include/asm/timex.h
> +++ b/arch/riscv/include/asm/timex.h
> @@ -33,6 +33,18 @@ static inline u32 get_cycles_hi(void)
> #define get_cycles_hi get_cycles_hi
> #endif /* CONFIG_64BIT */
>
> +/*
> + * Much like MIPS, we may not have a viable counter to use at an early point
> in
> + * the boot process. Unfortunately we don't have a fallback, so instead we
> + * just return 0.
> + */
> +static inline unsigned long random_get_entropy(void)
> +{
> + if (unlikely(clint_time_val == NULL))
> + return 0;
> + return get_cycles();
> +}
> +
Overall, this approach is good but this change is incomplete so does not work.
The linux/timex.h expects random_get_entropy() to be macro so we need a
"#define" as well.
After fixing rand_initialize() with custom random_get_entropy(), we get another
issue in boot_init_stack_canary() because boot_init_stack_canary() directly
calls get_cycles() so we remove use of get_cycles() from
boot_init_stack_canary()
and this is similar to ARM, ARM64, and MIPS kernel.
Regards,
Anup
> #else /* CONFIG_RISCV_M_MODE */
>
> static inline cycles_t get_cycles(void)
>
> >> Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
> >> for M-mode systems")
> >> Signed-off-by: Anup Patel
> >> ---
> >> Changes since v1:
> >> - Explicitly initialize clint_time_val to NULL in CLINT driver to
> >>avoid hang on Kendryte K210
> >> ---
> >> arch/riscv/include/asm/timex.h| 12 +---
> >> drivers/clocksource/timer-clint.c | 2 +-
> >> 2 files changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/riscv/include/asm/timex.h
> >> b/arch/riscv/include/asm/timex.h
> >> index 7f659dda0032..6e7b04874755 100644
> >> --- a/arch/riscv/include/asm/timex.h
> >> +++ b/arch/riscv/include/asm/timex.h
> >> @@ -17,18 +17,24 @@ typedef unsigned long cycles_t;
> >> #ifdef CONFIG_64BIT
> >> static inline cycles_t get_cycles(void)
> >> {
> >> -return readq_relaxed(clint_time_val);
> >> +if (clint_time_val)
> >> +return readq_relaxed(clint_time_val);
> >> +return 0;
> >> }
> >> #else /* !CONFIG_64BIT */
> >> static inline u32 get_cycles(void)
> >> {
> >> -return readl_relaxed(((u32 *)clint_time_val));
> >> +if (clint_time_val)
> >> +return readl_relaxed(((u32 *)clint_time_val));
> >> +return 0;
> >> }
> >> #define get_cycles get_cycles
> >>
> >> static inline u32 get_cycles_hi(void)
> >> {
> >> -return readl_relaxed(((u32 *)clint_time_val) + 1);
> >> +if (clint_time_val)
> >> +return readl_relaxed(((u32 *)clint_time_val) + 1);
> >> +return 0;
> >> }
> >> #define get_cycles_hi get_cycles_hi
> >> #endif /* CONFIG_64BIT */
> >> diff --git a/drivers/clocksource/timer-clint.c
> >> b/drivers/clocksource/timer-clint.c
> >> index d17367dee02c..8dbec85979fd 100644
> >> --- a/drivers/clocksource/timer-clint.c
> >> +++ b/drivers/clocksource/timer-clint.c
> >> @@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
> >> static unsigned int clint_timer_irq;
> >>
> >> #ifdef CONFIG_RISCV_M_MODE
> >> -u64 __iomem *clint_time_val;
> >> +u64 __iomem *clint_time_val = NULL;
>
> This one I definately don't get. According the internet, the C standard says
>
> If an object that has static storage duration is not initialized
> explicitly, it is initialized implicitly as if every member that has
> arithmetic type were assigned 0 and every member that has pointer type
> were
> assigned a null pointer constant.
>
> so unless I'm missing something there shouldn't be any difference between
> these
> two lines. When I just apply this I get exactly the same "objdump -dt" before
> and after. I do see some difference in assembly, but only when I don't pass
> "-fno-common" and that ends up being passed during my Linux builds.
>
> >> #endif
> >>
> >> static void clint_send_ipi(const struct cpumask *target)
> >
> > For Kendryte:
> >
> >
Re: [PATCH] mm: swapfile: avoid split_swap_cluster() NULL pointer dereference
Rafael Aquini writes:
> On Fri, Sep 25, 2020 at 11:21:58AM +0800, Huang, Ying wrote:
>> Rafael Aquini writes:
>> >> Or, can you help to run the test with a debug kernel based on upstream
>> >> kernel. I can provide some debug patch.
>> >>
>> >
>> > Sure, I can set your patches to run with the test cases we have that tend
>> > to
>> > reproduce the issue with some degree of success.
>>
>> Thanks!
>>
>> I found a race condition. During THP splitting, "head" may be unlocked
>> before calling split_swap_cluster(), because head != page during
>> deferred splitting. So we should call split_swap_cluster() before
>> unlocking. The debug patch to do that is as below. Can you help to
>> test it?
>>
>
>
> I finally could grab a good crashdump and confirm that head is really
> not locked.
Thanks! That's really helpful for us to root cause the bug.
> I still need to dig into it to figure out more about the
> crash. I guess that your patch will guarantee that lock on head, but
> it still doesn't help on explaining how did we get the THP marked as
> PG_swapcache, given that it should fail add_to_swap()->get_swap_page()
> right?
Because ClearPageCompound(head) is called in __split_huge_page(), then
all subpages except "page" are unlocked. So previously, when
split_swap_cluster() is called in split_huge_page_to_list(), the THP has
been split already and "head" may be unlocked. Then the normal page
"head" can be added to swap cache.
CPU1 CPU2
deferred_split_scan()
split_huge_page(page) /* page isn't compound head */
split_huge_page_to_list(page, NULL)
__split_huge_page(page, )
ClearPageCompound(head)
/* unlock all subpages except page (not head) */
add_to_swap(head) /* not THP */
get_swap_page(head)
add_to_swap_cache(head, )
SetPageSwapCache(head)
if PageSwapCache(head)
split_swap_cluster(/* swap entry of head */)
/* Deref sis->cluster_info: NULL accessing! */
> I'll give your patch a run over the weekend, hopefully we'll have more
> info on this next week.
Thanks!
Best Regards,
Huang, Ying
>> Best Regards,
>> Huang, Ying
>>
>> 8<
>> From 24ce0736a9f587d2dba12f12491c88d3e296a491 Mon Sep 17 00:00:00 2001
>> From: Huang Ying
>> Date: Fri, 25 Sep 2020 11:10:56 +0800
>> Subject: [PATCH] dbg: Call split_swap_clsuter() before unlock page during
>> split THP
>>
>> ---
>> mm/huge_memory.c | 13 +++--
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index faadc449cca5..8d79e5e6b46e 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2444,6 +2444,12 @@ static void __split_huge_page(struct page *page,
>> struct list_head *list,
>>
>> remap_page(head);
>>
>> +if (PageSwapCache(head)) {
>> +swp_entry_t entry = { .val = page_private(head) };
>> +
>> +split_swap_cluster(entry);
>> +}
>> +
>> for (i = 0; i < HPAGE_PMD_NR; i++) {
>> struct page *subpage = head + i;
>> if (subpage == page)
>> @@ -2678,12 +2684,7 @@ int split_huge_page_to_list(struct page *page, struct
>> list_head *list)
>> }
>>
>> __split_huge_page(page, list, end, flags);
>> -if (PageSwapCache(head)) {
>> -swp_entry_t entry = { .val = page_private(head) };
>> -
>> -ret = split_swap_cluster(entry);
>> -} else
>> -ret = 0;
>> +ret = 0;
>> } else {
>> if (IS_ENABLED(CONFIG_DEBUG_VM) && mapcount) {
>> pr_alert("total_mapcount: %u, page_count(): %u\n",
>> --
>> 2.28.0
>>
[PATCH 5/5 V3] PCI/ERR: don't mix io state not changed and no driver together
When we see 'can't recover (no error_detected callback)' on console,
Maybe the reason is io state is not changed by calling
pci_dev_set_io_state(), that is confused. fix it.
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
---
Chagnes:
V2: no change.
V3: no change.
drivers/pci/pcie/err.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index e35c4480c86b..d85f27c90c26 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -55,8 +55,10 @@ static int report_error_detected(struct pci_dev *dev,
if (!pci_dev_get(dev))
return 0;
device_lock(>dev);
- if (!pci_dev_set_io_state(dev, state) ||
- !dev->driver ||
+ if (!pci_dev_set_io_state(dev, state)) {
+ pci_dbg(dev, "Device might already being in error handling
...\n");
+ vote = PCI_ERS_RESULT_NONE;
+ } else if (!dev->driver ||
!dev->driver->err_handler ||
!dev->driver->err_handler->error_detected) {
/*
--
2.18.4
[PATCH 3/5 V3] PCI/ERR: get device before call device driver to avoid NULL pointer reference
During DPC error injection test we found there is race condition between
pciehp and DPC driver, NULL pointer reference caused panic as following
# setpci -s 64:02.0 0x196.w=000a
// 64:02.0 is rootport has DPC capability
# setpci -s 65:00.0 0x04.w=0544
// 65:00.0 is NVMe SSD populated in above port
# mount /dev/nvme0n1p1 nvme
(tested on stable 5.8 & ICS(Ice Lake SP platform, see
https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server))
Buffer I/O error on dev nvme0n1p1, logical block 468843328,
async page read
BUG: kernel NULL pointer dereference, address: 0050
#PF: supervisor read access in kernel mode
#PF: error_code(0x) - not-present page
PGD 0
Oops: [#1] SMP NOPTI
CPU: 12 PID: 513 Comm: irq/124-pcie-dp Not tainted 5.8.0-0.0.7.el8.x86_64+ #1
RIP: 0010:report_error_detected.cold.4+0x7d/0xe6
Code: b6 d0 e8 e8 fe 11 00 e8 16 c5 fb ff be 06 00 00 00 48 89 df e8 d3 65 ff
ff b8 06 00 00 00 e9 75 fc ff ff 48 8b 43 68 45 31 c9 <48> 8b 50 50 48 83 3a 00
41 0f 94 c1 45 31 c0 48 85 d2 41 0f 94 c0
RSP: 0018:ff8e06cf8762fda8 EFLAGS: 00010246
RAX: RBX: ff4e3eaacf42a000 RCX: ff4e3eb31f223c01
RDX: ff4e3eaacf42a140 RSI: ff4e3eb31f223c00 RDI: ff4e3eaacf42a138
RBP: ff8e06cf8762fdd0 R08: 00bf R09:
R10: 00eb8ebeab53 R11: 93453258 R12: 0002
R13: ff4e3eaacf42a130 R14: ff8e06cf8762fe2c R15: ff4e3eab44733828
FS: () GS:ff4e3eab1fd0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 0050 CR3: 000f8f80a004 CR4: 00761ee0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
PKRU: 5554
Call Trace:
? report_normal_detected+0x20/0x20
report_frozen_detected+0x16/0x20
pci_walk_bus+0x75/0x90
? dpc_irq+0x90/0x90
pcie_do_recovery+0x157/0x201
? irq_finalize_oneshot.part.47+0xe0/0xe0
dpc_handler+0x29/0x40
irq_thread_fn+0x24/0x60
irq_thread+0xea/0x170
? irq_forced_thread_fn+0x80/0x80
? irq_thread_check_affinity+0xf0/0xf0
kthread+0x124/0x140
? kthread_park+0x90/0x90
ret_from_fork+0x1f/0x30
Modules linked in: nft_fib_inet.
CR2: 0050
Though we partly close the race condition with patch 'PCI: pciehp: check
and wait port status out of DPC before handling DLLSC and PDC', but there
is no hardware spec or software sequence to guarantee the pcie_ist() run
into pci_wait_port_outdpc() first or DPC triggered status bits being set
first when errors triggered DPC containment procedure, so device still
could be removed by function pci_stop_and_removed_bus_device() then freed
by pci_dev_put() in pciehp driver first during pcie_do_recover()/
pci_walk_bus() is called by dpc_handler() in DPC driver.
Maybe unify pci_bus_sem and pci_rescan_remove_lock to serialize the
removal and walking operation is the right way, but here we use
pci_dev_get() to increase the reference count of device before using the
device to avoid it is freed in use.
With this patch and patch 'PCI: pciehp: check and wait port status out of
DPC before handling DLLSC and PDC', stable 5.9-rc6 could pass the error
injection test and no panic happened.
Brute DPC error injection script:
for i in {0..100}
do
setpci -s 64:02.0 0x196.w=000a
setpci -s 65:00.0 0x04.w=0544
mount /dev/nvme0n1p1 /root/nvme
sleep 1
done
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
Reviewed-by: Andy Shevchenko
---
Changes:
V2: revise doc according to Andy's suggestion.
V3: no change.
drivers/pci/pcie/err.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index c543f419d8f9..e35c4480c86b 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -52,6 +52,8 @@ static int report_error_detected(struct pci_dev *dev,
pci_ers_result_t vote;
const struct pci_error_handlers *err_handler;
+ if (!pci_dev_get(dev))
+ return 0;
device_lock(>dev);
if (!pci_dev_set_io_state(dev, state) ||
!dev->driver ||
@@ -76,6 +78,7 @@ static int report_error_detected(struct pci_dev *dev,
pci_uevent_ers(dev, vote);
*result = merge_result(*result, vote);
device_unlock(>dev);
+ pci_dev_put(dev);
return 0;
}
@@ -94,6 +97,8 @@ static int report_mmio_enabled(struct pci_dev *dev, void
*data)
pci_ers_result_t vote, *result = data;
const struct pci_error_handlers *err_handler;
+ if (!pci_dev_get(dev))
+ return 0;
device_lock(>dev);
if (!dev->driver ||
!dev->driver->err_handler ||
@@ -105,6 +110,7 @@ static int report_mmio_enabled(struct pci_dev *dev, void
*data)
*result = merge_result(*result, vote);
out:
device_unlock(>dev);
+ pci_dev_put(dev);
return 0;
}
@@ -113,6
[PATCH 4/5 V3] PCI: only return true when dev io state is really changed
When uncorrectable error happens, AER driver and DPC driver interrupt
handlers likely call
pcie_do_recovery()
->pci_walk_bus()
->report_frozen_detected()
with pci_channel_io_frozen the same time.
If pci_dev_set_io_state() return true even if the original state is
pci_channel_io_frozen, that will cause AER or DPC handler re-enter
the error detecting and recovery procedure one after another.
The result is the recovery flow mixed between AER and DPC.
So simplify the pci_dev_set_io_state() function to only return true
when dev->error_state is changed.
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
Reviewed-by: Andy Shevchenko
Reviewed-by: Alexandru Gagniuc
Reviewed-by: Joe Perches
---
Changnes:
V2: revise description and code according to suggestion from Andy.
V3: Change code to simpler.
drivers/pci/pci.h | 37 +
1 file changed, 5 insertions(+), 32 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index fa12f7cbc1a0..a2c1c7d5f494 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -359,39 +359,12 @@ struct pci_sriov {
static inline bool pci_dev_set_io_state(struct pci_dev *dev,
pci_channel_state_t new)
{
- bool changed = false;
-
device_lock_assert(>dev);
- switch (new) {
- case pci_channel_io_perm_failure:
- switch (dev->error_state) {
- case pci_channel_io_frozen:
- case pci_channel_io_normal:
- case pci_channel_io_perm_failure:
- changed = true;
- break;
- }
- break;
- case pci_channel_io_frozen:
- switch (dev->error_state) {
- case pci_channel_io_frozen:
- case pci_channel_io_normal:
- changed = true;
- break;
- }
- break;
- case pci_channel_io_normal:
- switch (dev->error_state) {
- case pci_channel_io_frozen:
- case pci_channel_io_normal:
- changed = true;
- break;
- }
- break;
- }
- if (changed)
- dev->error_state = new;
- return changed;
+ if (dev->error_state == new)
+ return false;
+
+ dev->error_state = new;
+ return true;
}
static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
--
2.18.4
[PATCH 0/5 V3] Fix DPC hotplug race and enhance error handling
This simple patch set fixed some serious security issues found when DPC
error injection and NVMe SSD hotplug brute force test were doing -- race
condition between DPC handler and pciehp, AER interrupt handlers, caused
system hang and system with DPC feature couldn't recover to normal
working state as expected (NVMe instance lost, mount operation hang,
race PCIe access caused uncorrectable errors reported alternatively etc).
With this patch set applied, stable 5.9-rc6 on ICS (Ice Lake SP platform,
see
https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server))
could pass the PCIe Gen4 NVMe SSD brute force hotplug test with any time
interval between hot-remove and plug-in operation tens of times without
any errors occur and system works normal.
With this patch set applied, system with DPC feature could recover from
NON-FATAL and FATAL errors injection test and works as expected.
System works smoothly when errors happen while hotplug is doing, no
uncorrectable errors found.
Brute DPC error injection script:
for i in {0..100}
do
setpci -s 64:02.0 0x196.w=000a
setpci -s 65:00.0 0x04.w=0544
mount /dev/nvme0n1p1 /root/nvme
sleep 1
done
Other details see every commits description part.
This patch set could be applied to stable 5.9-rc6 directly.
Help to review and test.
V2: changed according to review by Andy Shevchenko.
V3: changed patch 4/5 to simpler coding.
Thanks,
Ethan
Ethan Zhao (5):
PCI: define a function to check and wait till port finish DPC handling
PCI: pciehp: check and wait port status out of DPC before handling
DLLSC and PDC
PCI/ERR: get device before call device driver to avoid NULL pointer
reference
PCI: only return true when dev io state is really changed
PCI/ERR: don't mix io state not changed and no driver together
drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
drivers/pci/pci.h| 34 +---
drivers/pci/pcie/err.c | 18 +++--
include/linux/pci.h | 31 +
4 files changed, 55 insertions(+), 32 deletions(-)
--
2.18.4
[PATCH 2/5 V3] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC
When root port has DPC capability and it is enabled, then triggered by
errors, DPC DLLSC and PDC interrupts will be sent to DPC driver, pciehp
driver at the same time.
That will cause following result:
1. Link and device are recovered by hardware DPC and software DPC driver,
device
isn't removed, but the pciehp might treat it as device was hot removed.
2. Race condition happens bettween pciehp_unconfigure_device() called by
pciehp_ist() in pciehp driver and pci_do_recovery() called by
dpc_handler in DPC driver. no luck, there is no lock to protect
pci_stop_and_remove_bus_device()
against pci_walk_bus(), they hold different samphore and mutex,
pci_stop_and_remove_bus_device holds pci_rescan_remove_lock, and
pci_walk_bus() holds pci_bus_sem.
This race condition is not purely code analysis, it could be triggered by
following command series:
# setpci -s 64:02.0 0x196.w=000a // 64:02.0 rootport has DPC capability
# setpci -s 65:00.0 0x04.w=0544 // 65:00.0 NVMe SSD populated in port
# mount /dev/nvme0n1p1 nvme
One shot will cause system panic and NULL pointer reference happened.
(tested on stable 5.8 & ICS(Ice Lake SP platform, see
https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server))
Buffer I/O error on dev nvme0n1p1, logical block 3328, async page read
BUG: kernel NULL pointer dereference, address: 0050
#PF: supervisor read access in kernel mode
#PF: error_code(0x) - not-present page
PGD 0
Oops: [#1] SMP NOPTI
CPU: 12 PID: 513 Comm: irq/124-pcie-dp Not tainted 5.8.0 el8.x86_64+ #1
RIP: 0010:report_error_detected.cold.4+0x7d/0xe6
Code: b6 d0 e8 e8 fe 11 00 e8 16 c5 fb ff be 06 00 00 00 48 89 df e8 d3
65 ff ff b8 06 00 00 00 e9 75 fc ff ff 48 8b 43 68 45 31 c9 <48> 8b 50
50 48 83 3a 00 41 0f 94 c1 45 31 c0 48 85 d2 41 0f 94 c0
RSP: 0018:ff8e06cf8762fda8 EFLAGS: 00010246
RAX: RBX: ff4e3eaacf42a000 RCX: ff4e3eb31f223c01
RDX: ff4e3eaacf42a140 RSI: ff4e3eb31f223c00 RDI: ff4e3eaacf42a138
RBP: ff8e06cf8762fdd0 R08: 00bf R09:
R10: 00eb8ebeab53 R11: 93453258 R12: 0002
R13: ff4e3eaacf42a130 R14: ff8e06cf8762fe2c R15: ff4e3eab44733828
FS: () GS:ff4e3eab1fd0() knl
GS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 0050 CR3: 000f8f80a004 CR4: 00761ee0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
PKRU: 5554
Call Trace:
? report_normal_detected+0x20/0x20
report_frozen_detected+0x16/0x20
pci_walk_bus+0x75/0x90
? dpc_irq+0x90/0x90
pcie_do_recovery+0x157/0x201
? irq_finalize_oneshot.part.47+0xe0/0xe0
dpc_handler+0x29/0x40
irq_thread_fn+0x24/0x60
irq_thread+0xea/0x170
? irq_forced_thread_fn+0x80/0x80
? irq_thread_check_affinity+0xf0/0xf0
kthread+0x124/0x140
? kthread_park+0x90/0x90
ret_from_fork+0x1f/0x30
Modules linked in: nft_fib_inet.
CR2: 0050
With this patch, the handling flow of DPC containment and hotplug is
partly ordered and serialized, let hardware DPC do the controller reset
etc recovery action first, then DPC driver handling the call-back from
device drivers, clear the DPC status, at the end, pciehp handle the DLLSC
and PDC etc.
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
Reviewed-by: Andy Shevchenko
---
Changes:
V2: revise doc according to Andy's suggestion.
V3: no change.
drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 53433b37e181..6f271160f18d 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -710,8 +710,10 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
down_read(>reset_lock);
if (events & DISABLE_SLOT)
pciehp_handle_disable_request(ctrl);
- else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC))
+ else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) {
+ pci_wait_port_outdpc(pdev);
pciehp_handle_presence_or_link_change(ctrl, events);
+ }
up_read(>reset_lock);
ret = IRQ_HANDLED;
--
2.18.4
[PATCH 1/5 V3] PCI: define a function to check and wait till port finish DPC handling
Once root port DPC capability is enabled and triggered, at the beginning
of DPC is triggered, the DPC status bits are set by hardware and then
sends DPC/DLLSC/PDC interrupts to OS DPC and pciehp drivers, it will
take the port and software DPC interrupt handler 10ms to 50ms (test data
on ICS(Ice Lake SP platform, see
https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server)
& stable 5.9-rc6) to complete the DPC containment procedure
till the DPC status is cleared at the end of the DPC interrupt handler.
We use this function to check if the root port is in DPC handling status
and wait till the hardware and software completed the procedure.
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
Reviewed-by: Andy Shevchenko
---
changes:
V2：align ICS code name to public doc.
V3: no change.
include/linux/pci.h | 31 +++
1 file changed, 31 insertions(+)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 835530605c0d..5beb76c6ae26 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
@@ -2427,4 +2428,34 @@ void pci_uevent_ers(struct pci_dev *pdev, enum
pci_ers_result err_type);
WARN_ONCE(condition, "%s %s: " fmt, \
dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg)
+#ifdef CONFIG_PCIE_DPC
+static inline bool pci_wait_port_outdpc(struct pci_dev *pdev)
+{
+ u16 cap = pdev->dpc_cap, status;
+ u16 loop = 0;
+
+ if (!cap) {
+ pci_WARN_ONCE(pdev, !cap, "No DPC capability initiated\n");
+ return false;
+ }
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, );
+ pci_dbg(pdev, "DPC status %x, cap %x\n", status, cap);
+ while (status & PCI_EXP_DPC_STATUS_TRIGGER && loop < 100) {
+ msleep(10);
+ loop++;
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, );
+ }
+ if (!(status & PCI_EXP_DPC_STATUS_TRIGGER)) {
+ pci_dbg(pdev, "Out of DPC %x, cost %d ms\n", status, loop*10);
+ return true;
+ }
+ pci_dbg(pdev, "Timeout to wait port out of DPC status\n");
+ return false;
+}
+#else
+static inline bool pci_wait_port_outdpc(struct pci_dev *pdev)
+{
+ return true;
+}
+#endif
#endif /* LINUX_PCI_H */
--
2.18.4
RE: [PATCH 4/5 V2] PCI: only return true when dev io state is really changed
definitely simpler ! -Original Message- From: Joe Perches Sent: Sunday, September 27, 2020 12:17 PM To: Zhao, Haifeng ; bhelg...@google.com; ooh...@gmail.com; rus...@russell.cc; lu...@wunner.de; andriy.shevche...@linux.intel.com; stuart.w.ha...@gmail.com; mr.nuke...@gmail.com; mika.westerb...@linux.intel.com Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Jia, Pei P ; ashok@linux.intel.com; Kuppuswamy, Sathyanarayanan Subject: Re: [PATCH 4/5 V2] PCI: only return true when dev io state is really changed On Sat, 2020-09-26 at 23:28 -0400, Ethan Zhao wrote: > simplify the pci_dev_set_io_state() function to only return true when > dev->error_state is changed. [] > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h [] > @@ -362,35 +362,11 @@ static inline bool pci_dev_set_io_state(struct pci_dev > *dev, > bool changed = false; [] > + if (dev->error_state == new) > + return changed; > + > + dev->error_state = new; > + changed = true; > return changed; > } This would be simpler removing the unnecessary changed automatic ... if (dev->error_state == new) return false; dev->error_state = new; return true; }
RE: [PATCH 0/5] Add noncoherent platform support for vop driver
Hi Arnd, > Subject: Re: [PATCH 0/5] Add noncoherent platform support for vop driver > > On Fri, Sep 25, 2020 at 9:27 AM Sherry Sun wrote: > > > > Change the way of allocating vring to support noncoherent platform for > > vop driver, and add some related dma changes to make sure noncoherent > > platform works well. > > Could you describe why you are doing this? Are you using Intel MIC devices > on Arm hosts, or trying to reuse the code for other add-on cards? > We want to reuse the vop driver between two i.MX boards which are arm64 architecture. And in fact we have successfully verified it. But the biggest problem we currently encounter is about the noncoherent memory. Since the hardware device of our platform is not dma coherent, when use the original way(__get_free_pages and dma_map_single, but without dma_sync_single_for_cpu/device) will meet errors. Device pages/vring which are interact between ep and rc should use consistent memory without caching effects, so allocate them by dma_alloc_coherent is a better way. > Note that we have a couple of frameworks in the kernel that try to do some > of the same things here, notably the NTB drivers and the PCI endpoint > support, both of which are designed to be somewhat more generic than the > MIC driver. > > Have you considered using that instead? > > Arnd > Sorry I don't much about NTB, but for PCI endpoint driver, we will use it for pci data interaction below the vop layer. Best regards Sherry > > Sherry Sun (5): > > misc: vop: change the way of allocating vring for noncoherent platform > > misc: vop: change the way of allocating used ring > > misc: vop: simply return the saved dma address instead of virt_to_phys > > misc: vop: set VIRTIO_F_ACCESS_PLATFORM for nocoherent platform > > misc: vop: mapping kernel memory to user space as noncached > > > > drivers/misc/mic/bus/vop_bus.h| 2 + > > drivers/misc/mic/host/mic_boot.c | 8 ++ > > drivers/misc/mic/vop/vop_main.c | 51 + > > drivers/misc/mic/vop/vop_vringh.c | 117 > > -- > > 4 files changed, 125 insertions(+), 53 deletions(-) > > > > -- > > 2.17.1 > >
[PATCH 2/5 V2] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC
When root port has DPC capability and it is enabled, then triggered by
errors, DPC DLLSC and PDC interrupts will be sent to DPC driver, pciehp
driver at the same time.
That will cause following result:
1. Link and device are recovered by hardware DPC and software DPC driver,
device
isn't removed, but the pciehp might treat it as device was hot removed.
2. Race condition happens bettween pciehp_unconfigure_device() called by
pciehp_ist() in pciehp driver and pci_do_recovery() called by
dpc_handler in DPC driver. no luck, there is no lock to protect
pci_stop_and_remove_bus_device()
against pci_walk_bus(), they hold different samphore and mutex,
pci_stop_and_remove_bus_device holds pci_rescan_remove_lock, and
pci_walk_bus() holds pci_bus_sem.
This race condition is not purely code analysis, it could be triggered by
following command series:
# setpci -s 64:02.0 0x196.w=000a // 64:02.0 rootport has DPC capability
# setpci -s 65:00.0 0x04.w=0544 // 65:00.0 NVMe SSD populated in port
# mount /dev/nvme0n1p1 nvme
One shot will cause system panic and NULL pointer reference happened.
(tested on stable 5.8 & ICS(Ice Lake SP platform, see
https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server))
Buffer I/O error on dev nvme0n1p1, logical block 3328, async page read
BUG: kernel NULL pointer dereference, address: 0050
#PF: supervisor read access in kernel mode
#PF: error_code(0x) - not-present page
PGD 0
Oops: [#1] SMP NOPTI
CPU: 12 PID: 513 Comm: irq/124-pcie-dp Not tainted 5.8.0 el8.x86_64+ #1
RIP: 0010:report_error_detected.cold.4+0x7d/0xe6
Code: b6 d0 e8 e8 fe 11 00 e8 16 c5 fb ff be 06 00 00 00 48 89 df e8 d3
65 ff ff b8 06 00 00 00 e9 75 fc ff ff 48 8b 43 68 45 31 c9 <48> 8b 50
50 48 83 3a 00 41 0f 94 c1 45 31 c0 48 85 d2 41 0f 94 c0
RSP: 0018:ff8e06cf8762fda8 EFLAGS: 00010246
RAX: RBX: ff4e3eaacf42a000 RCX: ff4e3eb31f223c01
RDX: ff4e3eaacf42a140 RSI: ff4e3eb31f223c00 RDI: ff4e3eaacf42a138
RBP: ff8e06cf8762fdd0 R08: 00bf R09:
R10: 00eb8ebeab53 R11: 93453258 R12: 0002
R13: ff4e3eaacf42a130 R14: ff8e06cf8762fe2c R15: ff4e3eab44733828
FS: () GS:ff4e3eab1fd0() knl
GS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 0050 CR3: 000f8f80a004 CR4: 00761ee0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
PKRU: 5554
Call Trace:
? report_normal_detected+0x20/0x20
report_frozen_detected+0x16/0x20
pci_walk_bus+0x75/0x90
? dpc_irq+0x90/0x90
pcie_do_recovery+0x157/0x201
? irq_finalize_oneshot.part.47+0xe0/0xe0
dpc_handler+0x29/0x40
irq_thread_fn+0x24/0x60
irq_thread+0xea/0x170
? irq_forced_thread_fn+0x80/0x80
? irq_thread_check_affinity+0xf0/0xf0
kthread+0x124/0x140
? kthread_park+0x90/0x90
ret_from_fork+0x1f/0x30
Modules linked in: nft_fib_inet.
CR2: 0050
With this patch, the handling flow of DPC containment and hotplug is
partly ordered and serialized, let hardware DPC do the controller reset
etc recovery action first, then DPC driver handling the call-back from
device drivers, clear the DPC status, at the end, pciehp handle the DLLSC
and PDC etc.
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
Reviewed-by: Andy Shevchenko
---
Changes:
V2: revise doc according to Andy's suggestion.
drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 53433b37e181..6f271160f18d 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -710,8 +710,10 @@ static irqreturn_t pciehp_ist(int irq, void *dev_id)
down_read(>reset_lock);
if (events & DISABLE_SLOT)
pciehp_handle_disable_request(ctrl);
- else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC))
+ else if (events & (PCI_EXP_SLTSTA_PDC | PCI_EXP_SLTSTA_DLLSC)) {
+ pci_wait_port_outdpc(pdev);
pciehp_handle_presence_or_link_change(ctrl, events);
+ }
up_read(>reset_lock);
ret = IRQ_HANDLED;
--
2.18.4
Re: [PATCH v2 3/3] scsi: megaraid_sas: simplify compat_ioctl handling
On Sat, 2020-09-26 at 23:19 +0200, Arnd Bergmann wrote: > On Sat, Sep 19, 2020 at 7:26 AM Christoph Hellwig wrote: > > On Fri, Sep 18, 2020 at 02:15:43PM +0200, Arnd Bergmann wrote: [] > > > > + return ioc; > > > +out: > > > + kfree(ioc); > > > + > > > + return ERR_PTR(err); > > > > spurious empty line. > > ok I suggest moving the blank line so it's between the return ioc and out: label.
drivers/devfreq/imx-bus.c:120:34: warning: unused variable 'imx_bus_of_match'
Hi Leonard,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: a1bffa48745afbb54cb4f873bba783b2ae8be042
commit: 5173a9756c8df9c387e04e49da0c4061951bbfec PM / devfreq: Add generic imx
bus scaling driver
date: 4 months ago
config: x86_64-randconfig-r033-20200927 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
a83eb048cb9a75da7a07a9d5318bbdbf54885c87)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5173a9756c8df9c387e04e49da0c4061951bbfec
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 5173a9756c8df9c387e04e49da0c4061951bbfec
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/devfreq/imx-bus.c:120:34: warning: unused variable
>> 'imx_bus_of_match' [-Wunused-const-variable]
static const struct of_device_id imx_bus_of_match[] = {
^
1 warning generated.
vim +/imx_bus_of_match +120 drivers/devfreq/imx-bus.c
119
> 120 static const struct of_device_id imx_bus_of_match[] = {
121 { .compatible = "fsl,imx8m-noc", },
122 { .compatible = "fsl,imx8m-nic", },
123 { /* sentinel */ },
124 };
125 MODULE_DEVICE_TABLE(of, imx_bus_of_match);
126
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
sound/soc/samsung/aries_wm8994.c:525:34: warning: unused variable 'samsung_wm8994_of_match'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: a1bffa48745afbb54cb4f873bba783b2ae8be042
commit: 7a3a7671fa6c7e90aff5f4242add2a40587b85ef ASoC: samsung: Add driver for
Aries boards
date: 3 months ago
config: x86_64-randconfig-a001-20200927 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
a83eb048cb9a75da7a07a9d5318bbdbf54885c87)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7a3a7671fa6c7e90aff5f4242add2a40587b85ef
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 7a3a7671fa6c7e90aff5f4242add2a40587b85ef
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> sound/soc/samsung/aries_wm8994.c:525:34: warning: unused variable
>> 'samsung_wm8994_of_match' [-Wunused-const-variable]
static const struct of_device_id samsung_wm8994_of_match[] = {
^
1 warning generated.
vim +/samsung_wm8994_of_match +525 sound/soc/samsung/aries_wm8994.c
524
> 525 static const struct of_device_id samsung_wm8994_of_match[] = {
526 {
527 .compatible = "samsung,fascinate4g-wm8994",
528 .data = _variant,
529 },
530 {
531 .compatible = "samsung,aries-wm8994",
532 .data = _variant,
533 },
534 { /* sentinel */ },
535 };
536 MODULE_DEVICE_TABLE(of, samsung_wm8994_of_match);
537
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH 4/5 V2] PCI: only return true when dev io state is really changed
On Sat, 2020-09-26 at 23:28 -0400, Ethan Zhao wrote: > simplify the pci_dev_set_io_state() function to only return true > when dev->error_state is changed. [] > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h [] > @@ -362,35 +362,11 @@ static inline bool pci_dev_set_io_state(struct pci_dev > *dev, > bool changed = false; [] > + if (dev->error_state == new) > + return changed; > + > + dev->error_state = new; > + changed = true; > return changed; > } This would be simpler removing the unnecessary changed automatic ... if (dev->error_state == new) return false; dev->error_state = new; return true; }
[PATCH] ASoC: q6afe-clocks: Reduce code duplication via macro
The existing macro uses duplicate the index value so
move the index into the macro to reduce any possible
copy/paste and typo defects.
Miscellanea:
o Neaten macro
Signed-off-by: Joe Perches
---
sound/soc/qcom/qdsp6/q6afe-clocks.c | 191 ++---
---
1 file changed, 93 insertions(+), 98 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6afe-clocks.c
b/sound/soc/qcom/qdsp6/q6afe-clocks.c
index 2967f4546af5..4d897d6dad56 100644
--- a/sound/soc/qcom/qdsp6/q6afe-clocks.c
+++ b/sound/soc/qcom/qdsp6/q6afe-clocks.c
@@ -11,27 +11,6 @@
#include
#include "q6afe.h"
-#define Q6AFE_CLK(id) &(struct q6afe_clk) {\
- .clk_id = id, \
- .afe_clk_id = Q6AFE_##id, \
- .name = #id,\
- .attributes = LPASS_CLK_ATTRIBUTE_COUPLE_NO, \
- .hw.init = &(struct clk_init_data) {\
- .ops = _q6afe_ops, \
- .name = #id,\
- }, \
- }
-
-#define Q6AFE_VOTE_CLK(id, blkid, n) &(struct q6afe_clk) { \
- .clk_id = id, \
- .afe_clk_id = blkid,\
- .name = #n, \
- .hw.init = &(struct clk_init_data) {\
- .ops = _vote_q6afe_ops, \
- .name = #id,\
- }, \
- }
-
struct q6afe_clk {
struct device *dev;
int clk_id;
@@ -119,84 +98,100 @@ static const struct clk_ops clk_vote_q6afe_ops = {
.unprepare = clk_unvote_q6afe_block,
};
+#define Q6AFE_CLK(id) \
+ [id] = &(struct q6afe_clk) {\
+ .clk_id = id, \
+ .afe_clk_id = Q6AFE_##id, \
+ .name = #id,\
+ .attributes = LPASS_CLK_ATTRIBUTE_COUPLE_NO,\
+ .hw.init = &(struct clk_init_data) {\
+ .ops = _q6afe_ops, \
+ .name = #id,\
+ }, \
+ }
+
+#define Q6AFE_VOTE_CLK(id, blkid, n) \
+ [id] = &(struct q6afe_clk) {\
+ .clk_id = id, \
+ .afe_clk_id = blkid,\
+ .name = #n, \
+ .hw.init = &(struct clk_init_data) {\
+ .ops = _vote_q6afe_ops, \
+ .name = #id,\
+ }, \
+ }
+
struct q6afe_clk *q6afe_clks[Q6AFE_MAX_CLK_ID] = {
- [LPASS_CLK_ID_PRI_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_PRI_MI2S_IBIT),
- [LPASS_CLK_ID_PRI_MI2S_EBIT] = Q6AFE_CLK(LPASS_CLK_ID_PRI_MI2S_EBIT),
- [LPASS_CLK_ID_SEC_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_SEC_MI2S_IBIT),
- [LPASS_CLK_ID_SEC_MI2S_EBIT] = Q6AFE_CLK(LPASS_CLK_ID_SEC_MI2S_EBIT),
- [LPASS_CLK_ID_TER_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_TER_MI2S_IBIT),
- [LPASS_CLK_ID_TER_MI2S_EBIT] = Q6AFE_CLK(LPASS_CLK_ID_TER_MI2S_EBIT),
- [LPASS_CLK_ID_QUAD_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_QUAD_MI2S_IBIT),
- [LPASS_CLK_ID_QUAD_MI2S_EBIT] = Q6AFE_CLK(LPASS_CLK_ID_QUAD_MI2S_EBIT),
- [LPASS_CLK_ID_SPEAKER_I2S_IBIT] =
- Q6AFE_CLK(LPASS_CLK_ID_SPEAKER_I2S_IBIT),
- [LPASS_CLK_ID_SPEAKER_I2S_EBIT] =
- Q6AFE_CLK(LPASS_CLK_ID_SPEAKER_I2S_EBIT),
- [LPASS_CLK_ID_SPEAKER_I2S_OSR] =
- Q6AFE_CLK(LPASS_CLK_ID_SPEAKER_I2S_OSR),
- [LPASS_CLK_ID_QUI_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_QUI_MI2S_IBIT),
- [LPASS_CLK_ID_QUI_MI2S_EBIT] = Q6AFE_CLK(LPASS_CLK_ID_QUI_MI2S_EBIT),
- [LPASS_CLK_ID_SEN_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_SEN_MI2S_IBIT),
- [LPASS_CLK_ID_SEN_MI2S_EBIT] = Q6AFE_CLK(LPASS_CLK_ID_SEN_MI2S_EBIT),
- [LPASS_CLK_ID_INT0_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_INT0_MI2S_IBIT),
- [LPASS_CLK_ID_INT1_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_INT1_MI2S_IBIT),
- [LPASS_CLK_ID_INT2_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_INT2_MI2S_IBIT),
- [LPASS_CLK_ID_INT3_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_INT3_MI2S_IBIT),
- [LPASS_CLK_ID_INT4_MI2S_IBIT] = Q6AFE_CLK(LPASS_CLK_ID_INT4_MI2S_IBIT),
-
[PATCH 3/5 V2] PCI/ERR: get device before call device driver to avoid NULL pointer reference
During DPC error injection test we found there is race condition between
pciehp and DPC driver, NULL pointer reference caused panic as following
# setpci -s 64:02.0 0x196.w=000a
// 64:02.0 is rootport has DPC capability
# setpci -s 65:00.0 0x04.w=0544
// 65:00.0 is NVMe SSD populated in above port
# mount /dev/nvme0n1p1 nvme
(tested on stable 5.8 & ICS(Ice Lake SP platform, see
https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server))
Buffer I/O error on dev nvme0n1p1, logical block 468843328,
async page read
BUG: kernel NULL pointer dereference, address: 0050
#PF: supervisor read access in kernel mode
#PF: error_code(0x) - not-present page
PGD 0
Oops: [#1] SMP NOPTI
CPU: 12 PID: 513 Comm: irq/124-pcie-dp Not tainted 5.8.0-0.0.7.el8.x86_64+ #1
RIP: 0010:report_error_detected.cold.4+0x7d/0xe6
Code: b6 d0 e8 e8 fe 11 00 e8 16 c5 fb ff be 06 00 00 00 48 89 df e8 d3 65 ff
ff b8 06 00 00 00 e9 75 fc ff ff 48 8b 43 68 45 31 c9 <48> 8b 50 50 48 83 3a 00
41 0f 94 c1 45 31 c0 48 85 d2 41 0f 94 c0
RSP: 0018:ff8e06cf8762fda8 EFLAGS: 00010246
RAX: RBX: ff4e3eaacf42a000 RCX: ff4e3eb31f223c01
RDX: ff4e3eaacf42a140 RSI: ff4e3eb31f223c00 RDI: ff4e3eaacf42a138
RBP: ff8e06cf8762fdd0 R08: 00bf R09:
R10: 00eb8ebeab53 R11: 93453258 R12: 0002
R13: ff4e3eaacf42a130 R14: ff8e06cf8762fe2c R15: ff4e3eab44733828
FS: () GS:ff4e3eab1fd0() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 0050 CR3: 000f8f80a004 CR4: 00761ee0
DR0: DR1: DR2:
DR3: DR6: fffe0ff0 DR7: 0400
PKRU: 5554
Call Trace:
? report_normal_detected+0x20/0x20
report_frozen_detected+0x16/0x20
pci_walk_bus+0x75/0x90
? dpc_irq+0x90/0x90
pcie_do_recovery+0x157/0x201
? irq_finalize_oneshot.part.47+0xe0/0xe0
dpc_handler+0x29/0x40
irq_thread_fn+0x24/0x60
irq_thread+0xea/0x170
? irq_forced_thread_fn+0x80/0x80
? irq_thread_check_affinity+0xf0/0xf0
kthread+0x124/0x140
? kthread_park+0x90/0x90
ret_from_fork+0x1f/0x30
Modules linked in: nft_fib_inet.
CR2: 0050
Though we partly close the race condition with patch 'PCI: pciehp: check
and wait port status out of DPC before handling DLLSC and PDC', but there
is no hardware spec or software sequence to guarantee the pcie_ist() run
into pci_wait_port_outdpc() first or DPC triggered status bits being set
first when errors triggered DPC containment procedure, so device still
could be removed by function pci_stop_and_removed_bus_device() then freed
by pci_dev_put() in pciehp driver first during pcie_do_recover()/
pci_walk_bus() is called by dpc_handler() in DPC driver.
Maybe unify pci_bus_sem and pci_rescan_remove_lock to serialize the
removal and walking operation is the right way, but here we use
pci_dev_get() to increase the reference count of device before using the
device to avoid it is freed in use.
With this patch and patch 'PCI: pciehp: check and wait port status out of
DPC before handling DLLSC and PDC', stable 5.9-rc6 could pass the error
injection test and no panic happened.
Brute DPC error injection script:
for i in {0..100}
do
setpci -s 64:02.0 0x196.w=000a
setpci -s 65:00.0 0x04.w=0544
mount /dev/nvme0n1p1 /root/nvme
sleep 1
done
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
Reviewed-by: Andy Shevchenko
---
Changes:
V2: revise doc according to Andy's suggestion.
drivers/pci/pcie/err.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index c543f419d8f9..e35c4480c86b 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -52,6 +52,8 @@ static int report_error_detected(struct pci_dev *dev,
pci_ers_result_t vote;
const struct pci_error_handlers *err_handler;
+ if (!pci_dev_get(dev))
+ return 0;
device_lock(>dev);
if (!pci_dev_set_io_state(dev, state) ||
!dev->driver ||
@@ -76,6 +78,7 @@ static int report_error_detected(struct pci_dev *dev,
pci_uevent_ers(dev, vote);
*result = merge_result(*result, vote);
device_unlock(>dev);
+ pci_dev_put(dev);
return 0;
}
@@ -94,6 +97,8 @@ static int report_mmio_enabled(struct pci_dev *dev, void
*data)
pci_ers_result_t vote, *result = data;
const struct pci_error_handlers *err_handler;
+ if (!pci_dev_get(dev))
+ return 0;
device_lock(>dev);
if (!dev->driver ||
!dev->driver->err_handler ||
@@ -105,6 +110,7 @@ static int report_mmio_enabled(struct pci_dev *dev, void
*data)
*result = merge_result(*result, vote);
out:
device_unlock(>dev);
+ pci_dev_put(dev);
return 0;
}
@@ -113,6 +119,8 @@ static
[PATCH 0/5] MHI changes for v5.10 - Take two
Hi Greg, This is the second set of MHI patches for v5.10. The summary is below: * Fixed the format specifier used in debugfs interface. The issue was identified by building for ARM32 machine. NOTE: I've sent this patch separately for review. * Removed the auto-start option for MHI channels. This is done to avoid receiving spurious uplink from MHI client device when the client driver is not up. The corresponding qrtr change is also included with Dave's ACK. * Moved MHI_MAX_MTU define out of internal header to global to use it in client drivers. Please consider merging! Thanks, Mani Hemant Kumar (1): bus: mhi: core: Move MHI_MAX_MTU to external header file Loic Poulain (3): bus: mhi: debugfs: Print channel context read-pointer bus: mhi: Remove auto-start option net: qrtr: Start MHI channels during init Manivannan Sadhasivam (1): bus: mhi: core: debugfs: Use correct format specifiers for addresses drivers/bus/mhi/core/debugfs.c | 15 --- drivers/bus/mhi/core/init.c | 9 - drivers/bus/mhi/core/internal.h | 2 -- include/linux/mhi.h | 5 +++-- net/qrtr/mhi.c | 5 + 5 files changed, 16 insertions(+), 20 deletions(-) -- 2.17.1
[PATCH 4/5] net: qrtr: Start MHI channels during init
From: Loic Poulain Start MHI device channels so that transfers can be performed. The MHI stack does not auto-start channels anymore. Signed-off-by: Loic Poulain Reviewed-by: Manivannan Sadhasivam Acked-by: David S. Miller Signed-off-by: Manivannan Sadhasivam --- net/qrtr/mhi.c | 5 + 1 file changed, 5 insertions(+) diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c index ff0c41467fc1..7100f0bac4c6 100644 --- a/net/qrtr/mhi.c +++ b/net/qrtr/mhi.c @@ -76,6 +76,11 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev, struct qrtr_mhi_dev *qdev; int rc; + /* start channels */ + rc = mhi_prepare_for_transfer(mhi_dev); + if (rc) + return rc; + qdev = devm_kzalloc(_dev->dev, sizeof(*qdev), GFP_KERNEL); if (!qdev) return -ENOMEM; -- 2.17.1
[PATCH 3/5] bus: mhi: Remove auto-start option
From: Loic Poulain
There is really no point having an auto-start for channels.
This is confusing for the device drivers, some have to enable the
channels, others don't have... and waste resources (e.g. pre allocated
buffers) that may never be used.
This is really up to the MHI device(channel) driver to manage the state
of its channels.
Signed-off-by: Loic Poulain
Reviewed-by: Manivannan Sadhasivam
Signed-off-by: Manivannan Sadhasivam
---
drivers/bus/mhi/core/init.c | 9 -
drivers/bus/mhi/core/internal.h | 1 -
include/linux/mhi.h | 2 --
3 files changed, 12 deletions(-)
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 799111baceba..ca08437dffd6 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -772,7 +772,6 @@ static int parse_ch_cfg(struct mhi_controller *mhi_cntrl,
mhi_chan->offload_ch = ch_cfg->offload_channel;
mhi_chan->db_cfg.reset_req = ch_cfg->doorbell_mode_switch;
mhi_chan->pre_alloc = ch_cfg->auto_queue;
- mhi_chan->auto_start = ch_cfg->auto_start;
/*
* If MHI host allocates buffers, then the channel direction
@@ -1177,11 +1176,6 @@ static int mhi_driver_probe(struct device *dev)
goto exit_probe;
ul_chan->xfer_cb = mhi_drv->ul_xfer_cb;
- if (ul_chan->auto_start) {
- ret = mhi_prepare_channel(mhi_cntrl, ul_chan);
- if (ret)
- goto exit_probe;
- }
}
ret = -EINVAL;
@@ -1215,9 +1209,6 @@ static int mhi_driver_probe(struct device *dev)
if (ret)
goto exit_probe;
- if (dl_chan && dl_chan->auto_start)
- mhi_prepare_channel(mhi_cntrl, dl_chan);
-
mhi_device_put(mhi_dev);
return ret;
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 7989269ddd96..33c23203c531 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -563,7 +563,6 @@ struct mhi_chan {
bool configured;
bool offload_ch;
bool pre_alloc;
- bool auto_start;
bool wake_capable;
};
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d4841e5a5f45..6522a4adc794 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -214,7 +214,6 @@ enum mhi_db_brst_mode {
* @offload_channel: The client manages the channel completely
* @doorbell_mode_switch: Channel switches to doorbell mode on M0 transition
* @auto_queue: Framework will automatically queue buffers for DL traffic
- * @auto_start: Automatically start (open) this channel
* @wake-capable: Channel capable of waking up the system
*/
struct mhi_channel_config {
@@ -232,7 +231,6 @@ struct mhi_channel_config {
bool offload_channel;
bool doorbell_mode_switch;
bool auto_queue;
- bool auto_start;
bool wake_capable;
};
--
2.17.1
[PATCH 5/5] bus: mhi: core: Move MHI_MAX_MTU to external header file
From: Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.
Signed-off-by: Hemant Kumar
Reviewed-by: Manivannan Sadhasivam
Signed-off-by: Manivannan Sadhasivam
---
drivers/bus/mhi/core/internal.h | 1 -
include/linux/mhi.h | 3 +++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 33c23203c531..3e41edae829c 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
#define CMD_EL_PER_RING128
#define PRIMARY_CMD_RING 0
#define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
#define MHI_RANDOM_U32_NONZERO(bmsk) (prandom_u32_max(bmsk) + 1)
enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 6522a4adc794..10ebda44ea3c 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
#include
#include
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
#define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
struct mhi_chan;
--
2.17.1
[PATCH 2/5] bus: mhi: debugfs: Print channel context read-pointer
From: Loic Poulain This value was missing in the channel debugfs output. Signed-off-by: Loic Poulain Reviewed-by: Manivannan Sadhasivam Signed-off-by: Manivannan Sadhasivam --- drivers/bus/mhi/core/debugfs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c index f50d73054db4..3a48801e01f4 100644 --- a/drivers/bus/mhi/core/debugfs.c +++ b/drivers/bus/mhi/core/debugfs.c @@ -115,8 +115,9 @@ static int mhi_debugfs_channels_show(struct seq_file *m, void *d) seq_printf(m, " type: 0x%x event ring: %u", chan_ctxt->chtype, chan_ctxt->erindex); - seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx", - chan_ctxt->rbase, chan_ctxt->rlen, chan_ctxt->wp); + seq_printf(m, " base: 0x%llx len: 0x%llx rp: 0x%llx wp: 0x%llx", + chan_ctxt->rbase, chan_ctxt->rlen, chan_ctxt->rp, + chan_ctxt->wp); seq_printf(m, " local rp: 0x%pK local wp: 0x%pK db: 0x%pad\n", ring->rp, ring->wp, -- 2.17.1
[PATCH 1/5] bus: mhi: core: debugfs: Use correct format specifiers for addresses
For exposing the addresses of read/write pointers and doorbell register,
let's use the correct format specifiers. This fixes the following issues
generated using W=1 build in ARM32 and reported by Kbuild bot:
All warnings (new ones prefixed by >>):
>> drivers/bus/mhi/core/debugfs.c:75:7: warning: format specifies type
>> 'unsigned long long' but the argument has type 'dma_addr_t' (aka 'unsigned
>> int') [-Wformat]
mhi_event->db_cfg.db_val);
^~~~
drivers/bus/mhi/core/debugfs.c:123:7: warning: format specifies type
'unsigned long long' but the argument has type 'dma_addr_t' (aka 'unsigned
int') [-Wformat]
mhi_chan->db_cfg.db_val);
^~~
2 warnings generated.
drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_events_show’:
drivers/bus/mhi/core/debugfs.c:74:51: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]
seq_printf(m, " local rp: 0x%llx db: 0x%pad\n", (u64)ring->rp,
^
drivers/bus/mhi/core/debugfs.c: In function ‘mhi_debugfs_channels_show’:
drivers/bus/mhi/core/debugfs.c:122:7: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]
(u64)ring->rp, (u64)ring->wp,
^
drivers/bus/mhi/core/debugfs.c:122:22: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]
(u64)ring->rp, (u64)ring->wp,
^
drivers/bus/mhi/core/debugfs.c:121:62: warning: format ‘%llx’ expects argument
of type ‘long long unsigned int’, but argument 5 has type ‘dma_addr_t {aka
unsigned int}’ [-Wformat=]
seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n",
~~~^
%x
drivers/bus/mhi/core/debugfs.c:123:7:
mhi_chan->db_cfg.db_val);
Reported-by: kernel test robot
Signed-off-by: Manivannan Sadhasivam
---
drivers/bus/mhi/core/debugfs.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/bus/mhi/core/debugfs.c b/drivers/bus/mhi/core/debugfs.c
index 53d05a8e168d..f50d73054db4 100644
--- a/drivers/bus/mhi/core/debugfs.c
+++ b/drivers/bus/mhi/core/debugfs.c
@@ -71,8 +71,8 @@ static int mhi_debugfs_events_show(struct seq_file *m, void
*d)
seq_printf(m, " rp: 0x%llx wp: 0x%llx", er_ctxt->rp,
er_ctxt->wp);
- seq_printf(m, " local rp: 0x%llx db: 0x%llx\n", (u64)ring->rp,
- mhi_event->db_cfg.db_val);
+ seq_printf(m, " local rp: 0x%pK db: 0x%pad\n", ring->rp,
+ _event->db_cfg.db_val);
}
return 0;
@@ -118,9 +118,9 @@ static int mhi_debugfs_channels_show(struct seq_file *m,
void *d)
seq_printf(m, " base: 0x%llx len: 0x%llx wp: 0x%llx",
chan_ctxt->rbase, chan_ctxt->rlen, chan_ctxt->wp);
- seq_printf(m, " local rp: 0x%llx local wp: 0x%llx db: 0x%llx\n",
- (u64)ring->rp, (u64)ring->wp,
- mhi_chan->db_cfg.db_val);
+ seq_printf(m, " local rp: 0x%pK local wp: 0x%pK db: 0x%pad\n",
+ ring->rp, ring->wp,
+ _chan->db_cfg.db_val);
}
return 0;
--
2.17.1
Re: [PATCH 5/5] perf: arm_spe: Decode SVE events
Hi Andre,
On Tue, Sep 22, 2020 at 11:12:25AM +0100, Andre Przywara wrote:
> The Scalable Vector Extension (SVE) is an ARMv8 architecture extension
> that introduces very long vector operations (up to 2048 bits).
> The SPE profiling feature can tag SVE instructions with additional
> properties like predication or the effective vector length.
>
> Decode the new operation type bits in the SPE decoder to allow the perf
> tool to correctly report about SVE instructions.
>
> Signed-off-by: Andre Przywara
> ---
> .../arm-spe-decoder/arm-spe-pkt-decoder.c | 48 ++-
> 1 file changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> index a033f34846a6..f0c369259554 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> @@ -372,8 +372,35 @@ int arm_spe_pkt_desc(const struct arm_spe_pkt *packet,
> char *buf,
> }
> case ARM_SPE_OP_TYPE:
> switch (idx) {
> - case 0: return snprintf(buf, buf_len, "%s", payload & 0x1 ?
> + case 0: {
> + size_t blen = buf_len;
> +
> + if ((payload & 0x89) == 0x08) {
> + ret = snprintf(buf, buf_len, "SVE");
> + buf += ret;
> + blen -= ret;
> + if (payload & 0x2)
> + ret = snprintf(buf, buf_len, " FP");
> + else
> + ret = snprintf(buf, buf_len, " INT");
> + buf += ret;
> + blen -= ret;
> + if (payload & 0x4) {
> + ret = snprintf(buf, buf_len, " PRED");
> + buf += ret;
> + blen -= ret;
> + }
> + /* Bits [7..4] encode the vector length */
> + ret = snprintf(buf, buf_len, " EVLEN%d",
> +32 << ((payload >> 4) & 0x7));
> + buf += ret;
> + blen -= ret;
> + return buf_len - blen;
> + }
> +
> + return snprintf(buf, buf_len, "%s", payload & 0x1 ?
> "COND-SELECT" : "INSN-OTHER");
> + }
> case 1: {
> size_t blen = buf_len;
>
> @@ -403,6 +430,25 @@ int arm_spe_pkt_desc(const struct arm_spe_pkt *packet,
> char *buf,
> ret = snprintf(buf, buf_len, " NV-SYSREG");
> buf += ret;
> blen -= ret;
> + } else if ((payload & 0x0a) == 0x08) {
> + ret = snprintf(buf, buf_len, " SVE");
> + buf += ret;
> + blen -= ret;
> + if (payload & 0x4) {
> + ret = snprintf(buf, buf_len, " PRED");
> + buf += ret;
> + blen -= ret;
> + }
> + if (payload & 0x80) {
> + ret = snprintf(buf, buf_len, " SG");
> + buf += ret;
> + blen -= ret;
> + }
> + /* Bits [7..4] encode the vector length */
> + ret = snprintf(buf, buf_len, " EVLEN%d",
> +32 << ((payload >> 4) & 0x7));
> + buf += ret;
> + blen -= ret;
The changes in this patch has been included in the patch [1].
So my summary for patches 02 ~ 05, except patch 04, other changes has
been included in the patch set "perf arm-spe: Refactor decoding &
dumping flow".
I'd like to add your patch 04 into the patch set "perf arm-spe:
Refactor decoding & dumping flow" and I will respin the patch set v2 on
the latest perf/core branch and send out to review.
For patch 01, you could continue to try to land it in the kernel.
(Maybe consolidate a bit with Wei?).
Do you think this is okay for you?
Thanks,
Leo
[1] https://lore.kernel.org/patchwork/patch/1288413/
> } else if (payload & 0x4) {
> ret = snprintf(buf, buf_len, " SIMD-FP");
> buf += ret;
> --
> 2.17.1
>
[PATCH 5/5 V2] PCI/ERR: don't mix io state not changed and no driver together
When we see 'can't recover (no error_detected callback)' on console,
Maybe the reason is io state is not changed by calling
pci_dev_set_io_state(), that is confused. fix it.
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
---
Chagnes:
V2: no change.
drivers/pci/pcie/err.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index e35c4480c86b..d85f27c90c26 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -55,8 +55,10 @@ static int report_error_detected(struct pci_dev *dev,
if (!pci_dev_get(dev))
return 0;
device_lock(>dev);
- if (!pci_dev_set_io_state(dev, state) ||
- !dev->driver ||
+ if (!pci_dev_set_io_state(dev, state)) {
+ pci_dbg(dev, "Device might already being in error handling
...\n");
+ vote = PCI_ERS_RESULT_NONE;
+ } else if (!dev->driver ||
!dev->driver->err_handler ||
!dev->driver->err_handler->error_detected) {
/*
--
2.18.4
[PATCH 4/5 V2] PCI: only return true when dev io state is really changed
When uncorrectable error happens, AER driver and DPC driver interrupt
handlers likely call
pcie_do_recovery()
->pci_walk_bus()
->report_frozen_detected()
with pci_channel_io_frozen the same time.
If pci_dev_set_io_state() return true even if the original state is
pci_channel_io_frozen, that will cause AER or DPC handler re-enter
the error detecting and recovery procedure one after another.
The result is the recovery flow mixed between AER and DPC.
So simplify the pci_dev_set_io_state() function to only return true
when dev->error_state is changed.
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
Reviewed-by: Andy Shevchenko
Reviewed-by: Alexandru Gagniuc
---
Changes:
V2: revise doc and code flow according to Andy's suggestion.
drivers/pci/pci.h | 34 +-
1 file changed, 5 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index fa12f7cbc1a0..387f891ce6a1 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -362,35 +362,11 @@ static inline bool pci_dev_set_io_state(struct pci_dev
*dev,
bool changed = false;
device_lock_assert(>dev);
- switch (new) {
- case pci_channel_io_perm_failure:
- switch (dev->error_state) {
- case pci_channel_io_frozen:
- case pci_channel_io_normal:
- case pci_channel_io_perm_failure:
- changed = true;
- break;
- }
- break;
- case pci_channel_io_frozen:
- switch (dev->error_state) {
- case pci_channel_io_frozen:
- case pci_channel_io_normal:
- changed = true;
- break;
- }
- break;
- case pci_channel_io_normal:
- switch (dev->error_state) {
- case pci_channel_io_frozen:
- case pci_channel_io_normal:
- changed = true;
- break;
- }
- break;
- }
- if (changed)
- dev->error_state = new;
+ if (dev->error_state == new)
+ return changed;
+
+ dev->error_state = new;
+ changed = true;
return changed;
}
--
2.18.4
[PATCH 1/5 V2] PCI: define a function to check and wait till port finish DPC handling
Once root port DPC capability is enabled and triggered, at the beginning
of DPC is triggered, the DPC status bits are set by hardware and then
sends DPC/DLLSC/PDC interrupts to OS DPC and pciehp drivers, it will
take the port and software DPC interrupt handler 10ms to 50ms (test data
on ICS(Ice Lake SP platform, see
https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server)
& stable 5.9-rc6) to complete the DPC containment procedure
till the DPC status is cleared at the end of the DPC interrupt handler.
We use this function to check if the root port is in DPC handling status
and wait till the hardware and software completed the procedure.
Signed-off-by: Ethan Zhao
Tested-by: Wen Jin
Tested-by: Shanshan Zhang
Reviewed-by: Andy Shevchenko
---
changes:
V2：align ICS code name to public doc.
include/linux/pci.h | 31 +++
1 file changed, 31 insertions(+)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 835530605c0d..5beb76c6ae26 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -38,6 +38,7 @@
#include
#include
#include
+#include
#include
#include
@@ -2427,4 +2428,34 @@ void pci_uevent_ers(struct pci_dev *pdev, enum
pci_ers_result err_type);
WARN_ONCE(condition, "%s %s: " fmt, \
dev_driver_string(&(pdev)->dev), pci_name(pdev), ##arg)
+#ifdef CONFIG_PCIE_DPC
+static inline bool pci_wait_port_outdpc(struct pci_dev *pdev)
+{
+ u16 cap = pdev->dpc_cap, status;
+ u16 loop = 0;
+
+ if (!cap) {
+ pci_WARN_ONCE(pdev, !cap, "No DPC capability initiated\n");
+ return false;
+ }
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, );
+ pci_dbg(pdev, "DPC status %x, cap %x\n", status, cap);
+ while (status & PCI_EXP_DPC_STATUS_TRIGGER && loop < 100) {
+ msleep(10);
+ loop++;
+ pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, );
+ }
+ if (!(status & PCI_EXP_DPC_STATUS_TRIGGER)) {
+ pci_dbg(pdev, "Out of DPC %x, cost %d ms\n", status, loop*10);
+ return true;
+ }
+ pci_dbg(pdev, "Timeout to wait port out of DPC status\n");
+ return false;
+}
+#else
+static inline bool pci_wait_port_outdpc(struct pci_dev *pdev)
+{
+ return true;
+}
+#endif
#endif /* LINUX_PCI_H */
--
2.18.4
[PATCH 0/5 V2] Fix DPC hotplug race and enhance error handling
This simple patch set fixed some serious security issues found when DPC
error injection and NVMe SSD hotplug brute force test were doing -- race
condition between DPC handler and pciehp, AER interrupt handlers, caused
system hang and system with DPC feature couldn't recover to normal
working state as expected (NVMe instance lost, mount operation hang,
race PCIe access caused uncorrectable errors reported alternatively etc).
With this patch set applied, stable 5.9-rc6 on ICS (Ice Lake SP platform,
see
https://en.wikichip.org/wiki/intel/microarchitectures/ice_lake_(server))
could pass the PCIe Gen4 NVMe SSD brute force hotplug test with any time
interval between hot-remove and plug-in operation tens of times without
any errors occur and system works normal.
With this patch set applied, system with DPC feature could recover from
NON-FATAL and FATAL errors injection test and works as expected.
System works smoothly when errors happen while hotplug is doing, no
uncorrectable errors found.
Brute DPC error injection script:
for i in {0..100}
do
setpci -s 64:02.0 0x196.w=000a
setpci -s 65:00.0 0x04.w=0544
mount /dev/nvme0n1p1 /root/nvme
sleep 1
done
Other details see every commits description part.
This patch set could be applied to stable 5.9-rc6 directly.
Help to review and test.
V2: changed according to review by Andy Shevchenko.
Thanks,
Ethan
Ethan Zhao (5):
PCI: define a function to check and wait till port finish DPC handling
PCI: pciehp: check and wait port status out of DPC before handling
DLLSC and PDC
PCI/ERR: get device before call device driver to avoid NULL pointer
reference
PCI: only return true when dev io state is really changed
PCI/ERR: don't mix io state not changed and no driver together
drivers/pci/hotplug/pciehp_hpc.c | 4 +++-
drivers/pci/pci.h| 34 +---
drivers/pci/pcie/err.c | 18 +++--
include/linux/pci.h | 31 +
4 files changed, 55 insertions(+), 32 deletions(-)
--
2.18.4
Greetngs.......
Dear Friend, I am Barrister Razak George Ahmed, a lawyer and personal attorney to the late business man from your country who has a similar name with you, I request your consent to present you as the next of kin with the following relatives in order to continue to release his unknown deposits to you which are estimated at $ 5.5 million USD. Kindly get back to me if you can handle this transaction with me. Thanks Barrister Razak George Ahmed
Re: [PATCH 4/5] perf: arm_spe: Decode memory tagging properties
On Tue, Sep 22, 2020 at 11:12:24AM +0100, Andre Przywara wrote:
> When SPE records a physical address, it can additionally tag the event
> with information from the Memory Tagging architecture extension.
>
> Decode the two additional fields in the SPE event payload.
>
> Signed-off-by: Andre Przywara
> ---
> .../util/arm-spe-decoder/arm-spe-pkt-decoder.c | 17 -
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> index 943e4155b246..a033f34846a6 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> @@ -8,13 +8,14 @@
> #include
> #include
> #include
> +#include
>
> #include "arm-spe-pkt-decoder.h"
>
> -#define BIT(n) (1ULL << (n))
> -
> #define NS_FLAG BIT(63)
> #define EL_FLAG (BIT(62) | BIT(61))
> +#define CH_FLAG BIT(62)
> +#define PAT_FLAG GENMASK_ULL(59, 56)
>
> #define SPE_HEADER0_PAD 0x0
> #define SPE_HEADER0_END 0x1
> @@ -447,10 +448,16 @@ int arm_spe_pkt_desc(const struct arm_spe_pkt *packet,
> char *buf,
> return snprintf(buf, buf_len, "%s 0x%llx el%d ns=%d",
> (idx == 1) ? "TGT" : "PC", payload, el,
> ns);
> case 2: return snprintf(buf, buf_len, "VA 0x%llx", payload);
> - case 3: ns = !!(packet->payload & NS_FLAG);
> + case 3: {
> + int ch = !!(packet->payload & CH_FLAG);
> + int pat = (packet->payload & PAT_FLAG) >> 56;
> +
> + ns = !!(packet->payload & NS_FLAG);
> payload &= ~(0xffULL << 56);
> - return snprintf(buf, buf_len, "PA 0x%llx ns=%d",
> - payload, ns);
> + return snprintf(buf, buf_len,
> + "PA 0x%llx ns=%d ch=%d, pat=%x",
> + payload, ns, ch, pat);
> + }
Reviewed-by: Leo Yan
> default: return 0;
> }
> case ARM_SPE_CONTEXT:
> --
> 2.17.1
>
[rcu:master] BUILD SUCCESS 6beb1792a233851a1ee0e555e9395909ed3ae7e9
allyesconfig cskydefconfig alpha defconfig alphaallyesconfig arc defconfig sh allmodconfig xtensa allyesconfig h8300allyesconfig parisc defconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a002-20200925 i386 randconfig-a006-20200925 i386 randconfig-a003-20200925 i386 randconfig-a004-20200925 i386 randconfig-a005-20200925 i386 randconfig-a001-20200925 i386 randconfig-a002-20200927 i386 randconfig-a006-20200927 i386 randconfig-a003-20200927 i386 randconfig-a004-20200927 i386 randconfig-a005-20200927 i386 randconfig-a001-20200927 i386 randconfig-a002-20200926 i386 randconfig-a006-20200926 i386 randconfig-a003-20200926 i386 randconfig-a004-20200926 i386 randconfig-a005-20200926 i386 randconfig-a001-20200926 x86_64 randconfig-a011-20200927 x86_64 randconfig-a013-20200927 x86_64 randconfig-a014-20200927 x86_64 randconfig-a015-20200927 x86_64 randconfig-a012-20200927 x86_64 randconfig-a016-20200927 x86_64 randconfig-a011-20200925 x86_64 randconfig-a013-20200925 x86_64 randconfig-a014-20200925 x86_64 randconfig-a015-20200925 x86_64 randconfig-a012-20200925 x86_64 randconfig-a016-20200925 i386 randconfig-a012-20200926 i386 randconfig-a014-20200926 i386 randconfig-a016-20200926 i386 randconfig-a013-20200926 i386 randconfig-a011-20200926 i386 randconfig-a015-20200926 i386 randconfig-a012-20200927 i386 randconfig-a014-20200927 i386 randconfig-a016-20200927 i386 randconfig-a013-20200927 i386 randconfig-a011-20200927 i386 randconfig-a015-20200927 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec clang tested configs: x86_64 randconfig-a005-20200927 x86_64 randconfig-a003-20200927 x86_64 randconfig-a004-20200927 x86_64 randconfig-a002-20200927 x86_64 randconfig-a006-20200927 x86_64 randconfig-a001-20200927 x86_64 randconfig-a011-20200926 x86_64 randconfig-a013-20200926 x86_64 randconfig-a014-20200926 x86_64 randconfig-a015-20200926 x86_64 randconfig-a012-20200926 x86_64 randconfig-a016-20200926 x86_64 randconfig-a005-20200925 x86_64 randconfig-a003-20200925 x86_64 randconfig-a004-20200925 x86_64 randconfig-a002-20200925 x86_64 randconfig-a006-20200925 x86_64 randconfig-a001-20200925 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
drivers/staging/media/rkvdec/rkvdec.c:967:34: warning: unused variable 'of_rkvdec_match'
Hi Boris,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: a1bffa48745afbb54cb4f873bba783b2ae8be042
commit: cd33c830448baf7b1e94da72eca069e3e1d050c9 media: rkvdec: Add the rkvdec
driver
date: 5 months ago
config: x86_64-randconfig-a001-20200927 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
a83eb048cb9a75da7a07a9d5318bbdbf54885c87)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=cd33c830448baf7b1e94da72eca069e3e1d050c9
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout cd33c830448baf7b1e94da72eca069e3e1d050c9
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/staging/media/rkvdec/rkvdec.c:967:34: warning: unused variable
>> 'of_rkvdec_match' [-Wunused-const-variable]
static const struct of_device_id of_rkvdec_match[] = {
^
1 warning generated.
vim +/of_rkvdec_match +967 drivers/staging/media/rkvdec/rkvdec.c
966
> 967 static const struct of_device_id of_rkvdec_match[] = {
968 { .compatible = "rockchip,rk3399-vdec" },
969 { /* sentinel */ }
970 };
971 MODULE_DEVICE_TABLE(of, of_rkvdec_match);
972
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v6 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file
On Wed, Sep 16, 2020 at 12:56:05PM -0700, Hemant Kumar wrote:
> Currently this macro is defined in internal MHI header as
> a TRE length mask. Moving it to external header allows MHI
> client drivers to set this upper bound for the transmit
> buffer size.
>
> Signed-off-by: Hemant Kumar
Reviewed-by: Manivannan Sadhasivam
Thanks,
Mani
> ---
> drivers/bus/mhi/core/internal.h | 1 -
> include/linux/mhi.h | 3 +++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
> index 7989269..4abf0cf 100644
> --- a/drivers/bus/mhi/core/internal.h
> +++ b/drivers/bus/mhi/core/internal.h
> @@ -453,7 +453,6 @@ enum mhi_pm_state {
> #define CMD_EL_PER_RING 128
> #define PRIMARY_CMD_RING 0
> #define MHI_DEV_WAKE_DB 127
> -#define MHI_MAX_MTU 0x
> #define MHI_RANDOM_U32_NONZERO(bmsk) (prandom_u32_max(bmsk) + 1)
>
> enum mhi_er_type {
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 6565528..610f3b0 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -16,6 +16,9 @@
> #include
> #include
>
> +/* MHI client drivers to set this upper bound for tx buffer */
> +#define MHI_MAX_MTU 0x
> +
> #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
>
> struct mhi_chan;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [PATCH v6 1/4] bus: mhi: core: Add helper API to return number of free TREs
On Wed, Sep 16, 2020 at 12:56:04PM -0700, Hemant Kumar wrote:
> Introduce mhi_get_no_free_descriptors() API to return number
> of TREs available to queue buffer. MHI clients can use this
> API to know before hand if ring is full without calling queue
> API.
>
> Signed-off-by: Hemant Kumar
> ---
> drivers/bus/mhi/core/main.c | 12
> include/linux/mhi.h | 9 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 2cff5dd..0599e7d 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -258,6 +258,18 @@ int mhi_destroy_device(struct device *dev, void *data)
> return 0;
> }
>
> +int mhi_get_no_free_descriptors(struct mhi_device *mhi_dev,
> + enum dma_data_direction dir)
> +{
> + struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> + struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
> + mhi_dev->ul_chan : mhi_dev->dl_chan;
> + struct mhi_ring *tre_ring = _chan->tre_ring;
> +
> + return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
Hmm, so this is essentially a wrapper for get_nr_avail_ring_elements().
Why can't you call this API directly?
> +}
> +EXPORT_SYMBOL_GPL(mhi_get_no_free_descriptors);
> +
> void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
> {
> struct mhi_driver *mhi_drv;
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index a35d876..6565528 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -600,6 +600,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
> void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
>
> /**
> + * mhi_get_no_free_descriptors - Get transfer ring length
mhi_get_nr_free_descriptors?
> + * Get # of TD available to queue buffers
> + * @mhi_dev: Device associated with the channels
> + * @dir: Direction of the channel
> + */
> +int mhi_get_no_free_descriptors(struct mhi_device *mhi_dev,
> + enum dma_data_direction dir);
Align enum with start of "("
Thanks,
Mani
> +
> +/**
> * mhi_prepare_for_power_up - Do pre-initialization before power up.
> *This is optional, call this before power up if
> *the controller does not want bus framework to
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
Re: [PATCH 3/5] perf: arm_spe: Add nested virt event decoding
On Tue, Sep 22, 2020 at 11:12:23AM +0100, Andre Przywara wrote:
> The ARMv8.4 nested virtualisation extension can redirect system register
> accesses to a memory page controlled by the hypervisor. The SPE
> profiling feature in newer implementations can tag those memory accesses
> accordingly.
>
> Add the bit pattern describing this load/store type, so that the perf
> tool can decode it properly.
>
> Signed-off-by: Andre Przywara
> ---
> tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> index e633bb5b8e65..943e4155b246 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> @@ -398,6 +398,10 @@ int arm_spe_pkt_desc(const struct arm_spe_pkt *packet,
> char *buf,
> buf += ret;
> blen -= ret;
> }
> + } else if ((payload & 0xfe) == 0x30) {
> + ret = snprintf(buf, buf_len, " NV-SYSREG");
> + buf += ret;
> + blen -= ret;
This change has been included in the patch "perf arm-spe: Add more sub
classes for operation packet" [1].
Thanks,
Leo
[1] https://lore.kernel.org/patchwork/patch/1288412/
> } else if (payload & 0x4) {
> ret = snprintf(buf, buf_len, " SIMD-FP");
> buf += ret;
> --
> 2.17.1
>
Re: [PATCH 2/5] perf: arm_spe: Add new event packet bits
On Tue, Sep 22, 2020 at 11:12:22AM +0100, Andre Przywara wrote:
> The ARMv8.3-SPE extension adds some new bits to the event packet
> fields.
>
> Handle bits 11 (alignment), 17 and 18 (SVE predication) when decoding
> the SPE buffer content.
>
> Signed-off-by: Andre Przywara
> ---
> .../util/arm-spe-decoder/arm-spe-pkt-decoder.c | 17 +
> 1 file changed, 17 insertions(+)
>
> diff --git a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> index b94001b756c7..e633bb5b8e65 100644
> --- a/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> +++ b/tools/perf/util/arm-spe-decoder/arm-spe-pkt-decoder.c
> @@ -346,6 +346,23 @@ int arm_spe_pkt_desc(const struct arm_spe_pkt *packet,
> char *buf,
> buf += ret;
> blen -= ret;
> }
> + if (payload & BIT(11)) {
> + ret = snprintf(buf, buf_len, " ALIGNMENT");
> + buf += ret;
> + blen -= ret;
> + }
> + }
> + if (idx > 2) {
> + if (payload & BIT(17)) {
> + ret = snprintf(buf, buf_len, "
> SVE-PARTIAL-PRED");
> + buf += ret;
> + blen -= ret;
> + }
> + if (payload & BIT(18)) {
> + ret = snprintf(buf, buf_len, " SVE-EMPTY-PRED");
> + buf += ret;
> + blen -= ret;
> + }
>From patch 02 to patch 05, some changes have been included in the
patch set "perf arm-spe: Refactor decoding & dumping flow". I
refactored the Arm SPE decoder so uses macros to replace the hard code
numbers for packet formats. So I'd like your changes could rebase on
this refactor patch set, thus can reuse the predefined macros for
decoding.
For this patch, it has been included in the patch [2]. You could see
your implementation is difference for handling "ALIGNMENT", it misses
to check "idx > 2". It would be very helpful if you could review
patch [2].
Thanks,
Leo
[1] https://lore.kernel.org/patchwork/cover/1288406/
[2] https://lore.kernel.org/patchwork/patch/1288413/
> }
> if (ret < 0)
> return ret;
> --
> 2.17.1
>
Re: [PATCH] ocfs2: fix potential soft lockup during fstrim
On 2020/9/27 09:58, Gang He wrote:
> When we discard unused blocks on a mounted ocfs2 filesystem, fstrim
> handles each block goup with locking/unlocking global bitmap meta-file
> repeatedly. we should let fstrim thread take a break(if need) between
> unlock and lock, this will avoid the potential soft lockup problem,
> and also gives the upper applications more IO opportunities, these
> applications are not blocked for too long at writing files.
>
> Signed-off-by: Gang He
It makes sense.
Reviewed-by: Joseph Qi
> ---
> fs/ocfs2/alloc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
> index 4c1b90442d6f..2cf9321919b5 100644
> --- a/fs/ocfs2/alloc.c
> +++ b/fs/ocfs2/alloc.c
> @@ -7654,8 +7654,10 @@ int ocfs2_trim_mainbm(struct super_block *sb, struct
> fstrim_range *range)
>* main_bm related locks for avoiding the current IO starve, then go to
>* trim the next group
>*/
> - if (ret >= 0 && group <= last_group)
> + if (ret >= 0 && group <= last_group) {
> + cond_resched();
> goto next_group;
> + }
> out:
> range->len = trimmed * sb->s_blocksize;
> return ret;
>
[PATCH v2 1/1] mmc: host: meson-gx-mmc: fix possible deadlock condition for preempt_rt
---
drivers/mmc/host/meson-gx-mmc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 08a3b1c05..3ba8f988d 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -146,6 +146,7 @@ struct sd_emmc_desc {
};
struct meson_host {
+ spinlock_t lock;
struct device *dev;
struct meson_mmc_data *data;
struct mmc_host*mmc;
@@ -1051,6 +1052,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
host->mmc = mmc;
host->dev = >dev;
dev_set_drvdata(>dev, host);
+ spin_lock_init(>lock);
/* The G12A SDIO Controller needs an SRAM bounce buffer */
host->dram_access_quirk = device_property_read_bool(>dev,
@@ -1139,7 +1141,7 @@ static int meson_mmc_probe(struct platform_device *pdev)
host->regs + SD_EMMC_IRQ_EN);
ret = request_threaded_irq(host->irq, meson_mmc_irq,
- meson_mmc_irq_thread, IRQF_ONESHOT,
+ meson_mmc_irq_thread, 0,
dev_name(>dev), host);
if (ret)
goto err_init_clk;
--
2.20.1
[PATCH v2 0/1] mmc: host: meson-gx-mmc: fix possible deadlock condition for preempt_rt
This is a updated experiamental patch for review following discussions
with Jerome / Sebastian regarding the usage of threadded interupts in
meson-gx-mmc. I don't have a complete understanding or am I a kernel
developer but this is my best efforts attempt to address this issue.
Also thanks to both of of them for opening up the discussions and
Kevin for pointing me in the right direction for patch formatting.
Force threaded interrupts for meson_mmc_irq to prevent possible deadlock
condition during mmc operations when using preempt_rt with 5.9.0-rc3-rt3
patches on arm64.
Using meson-gx-mmc with an emmc device on Hardkernel Odroid N2+
configured with preempt_rt resulted in the soc becoming unresponsive.
With lock checking enabled the below inconsistent lock state was
observed during boot.
After some discussions with tglx in IRC #linux-rt a patch was suggested
to remove IRQF_ONESHOT from request_threaded_irq.
This has been tested and confirmed by me to resolve both the
unresponsive soc and the inconsistent lock state warning when using
5.9.0-rc3-rt3 on arm64 Odroid N2+.
Further review and testing is required to ensure there are no adverse
impacts or concerns and that is the correct method to resolve the
problem. I will continue to test on various amlogic devices with both
standard mainline low latency kernel and preempt_rt kernel with -rt
patches.
Changes since v1:
- Add spinlock_t lock to meson_host structure
- Add spin_lock_init to driver probe for the host lock to ensure the
irq will not attempt to fire again if the threaded irq component
is not complete
[7.858446]
[7.858448] WARNING: inconsistent lock state
[7.858450] 5.9.0-rc3-rt3+ #33 Not tainted
[7.858453]
[7.858456] inconsistent {IN-HARDIRQ-R} -> {HARDIRQ-ON-W} usage.
[7.858459] swapper/0/1 [HC0[0]:SC0[0]:HE1:SE1] takes:
[7.858465] 80001219f4d8 (>leddev_list_lock){+?.+}-{0:0}, at:
led_trigger_set+0x104/0x270
[7.858482] {IN-HARDIRQ-R} state was registered at:
[7.858484] lock_acquire+0xec/0x468
[7.858491] rt_read_lock+0xb0/0x108
[7.858497] led_trigger_event+0x34/0x88
[7.858501] mmc_request_done+0x3f0/0x450
[7.858505] meson_mmc_irq+0x284/0x378
[7.858511] __handle_irq_event_percpu+0xcc/0x4a8
[7.858515] handle_irq_event_percpu+0x60/0xb0
[7.858519] handle_irq_event+0x50/0x108
[7.858522] handle_fasteoi_irq+0xd0/0x180
[7.858527] generic_handle_irq+0x38/0x50
[7.858530] __handle_domain_irq+0x6c/0xc8
[7.858533] gic_handle_irq+0x5c/0xb8
[7.858537] el1_irq+0xbc/0x180
[7.858540] arch_cpu_idle+0x28/0x38
[7.858544] default_idle_call+0x90/0x3f0
[7.858547] do_idle+0x250/0x268
[7.858551] cpu_startup_entry+0x2c/0x78
[7.858554] rest_init+0x1b0/0x284
[7.858559] arch_call_rest_init+0x18/0x24
[7.858565] start_kernel+0x550/0x588
[7.858569] irq event stamp: 1925495
[7.858571] hardirqs last enabled at (1925495): []
_raw_spin_unlock_irqrestore+0xa4/0xb0
[7.858576] hardirqs last disabled at (1925494): []
_raw_spin_lock_irqsave+0xa8/0xb8
[7.858580] softirqs last enabled at (1857856): []
bdi_register_va+0x114/0x368
[7.858586] softirqs last disabled at (1857849): []
bdi_register_va+0x114/0x368
[7.858590]
other info that might help us debug this:
[7.858592] Possible unsafe locking scenario:
[7.858594]CPU0
[7.858595]
[7.858597] lock(>leddev_list_lock);
[7.858600]
[7.858602] lock(>leddev_list_lock);
[7.858604]
*** DEADLOCK ***
[7.858606] 3 locks held by swapper/0/1:
[7.858609] #0: 80001219eb30 (leds_list_lock){}-{0:0}, at:
led_trigger_register+0xf4/0x1c0
[7.858619] #1: b0696a70 (_cdev->trigger_lock){+.+.}-{0:0}, at:
led_trigger_register+0x134/0x1c0
[7.858629] #2: 800011fb83d0 (rcu_read_lock){}-{1:2}, at:
rt_write_lock+0x8/0x108
[7.858637]
stack backtrace:
[7.858640] CPU: 5 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc3-rt3+ #33
[7.858643] Hardware name: Hardkernel ODROID-N2Plus (DT)
[7.858646] Call trace:
[7.858647] dump_backtrace+0x0/0x1e8
[7.858650] show_stack+0x20/0x30
[7.858653] dump_stack+0xf0/0x164
[7.858659] print_usage_bug+0x2b4/0x2c0
[7.858662] mark_lock+0x2e8/0x360
[7.858665] __lock_acquire+0x238/0x1858
[7.858669] lock_acquire+0xec/0x468
[7.858672] rt_write_lock+0xb0/0x108
[7.858675] led_trigger_set+0x104/0x270
[7.858678] led_trigger_register+0x180/0x1c0
[7.858681] heartbeat_trig_init+0x28/0x5c
[7.858686] do_one_initcall+0x90/0x4bc
[7.858690] kernel_init_freeable+0x2cc/0x338
[7.858694] kernel_init+0x1c/0x11c
[7.858697] ret_from_fork+0x10/0x34
Brad Harper (1):
mmc: host: meson-gx-mmc: fix possible deadlock condition for
preempt_rt
Re: [PATCH] Only allow to set crash_kexec_post_notifiers on boot time
Hi, On 09/25/20 at 10:56am, Konrad Rzeszutek Wilk wrote: > On Fri, Sep 25, 2020 at 11:05:58AM +0800, Dave Young wrote: > > Hi, > > > > On 09/24/20 at 01:16pm, boris.ostrov...@oracle.com wrote: > > > > > > On 9/24/20 12:43 PM, Michael Kelley wrote: > > > > From: Eric W. Biederman Sent: Thursday, > > > > September 24, 2020 9:26 AM > > > >> Michael Kelley writes: > > > >> > > > > Added Hyper-V people and people who created the param, it is below > > > > commit, I also want to remove it if possible, let's see how people > > > > think, but the least way should be to disable the auto setting in > > > > both systemd > > > > and kernel: > > > >>> Hyper-V uses a notifier to inform the host system that a Linux VM has > > > >>> panic'ed. Informing the host is particularly important in a public > > > >>> cloud > > > >>> such as Azure so that the cloud software can alert the customer, and > > > >>> can > > > >>> track cloud-wide reliability statistics. Whether a kdump is taken > > > >>> is controlled > > > >>> entirely by the customer and how he configures the VM, and we want > > > >>> the host to be informed either way. > > > >> Why? > > > >> > > > >> Why does the host care? > > > >> Especially if the VM continues executing into a kdump kernel? > > > > The host itself doesn't care. But the host is a convenient out-of-band > > > > channel for recording that a panic has occurred and to collect basic > > > > data > > > > about the panic. This out-of-band channel is then used to notify the > > > > end > > > > customer that his VM has panic'ed. Sure, the customer should be running > > > > his own monitoring software, but customers don't always do what they > > > > should. Equally important, the out-of-band channel allows the cloud > > > > infrastructure software to notice trends, such as that the rate of Linux > > > > panics has increased, and that perhaps there is a cloud problem that > > > > should be investigated. > > > > > > > > > In many cases (especially in cloud environment) your dump device is > > > remote (e.g. iscsi) and kdump sometimes (often?) gets stuck because of > > > connectivity issues (which could be cause of the panic in the first > > > place). So it is quite desirable to inform the infrastructure that the VM > > > is on its way out without waiting for kdump to complete. > > > > That can probably be done in kdump kernel if it is really needed. Say > > informing host that panic happened and a kdump kernel is runnning. > > If kdump kernel gets to that point. Sometimes (sadly) it ends up being > misconfigured and it chokes up - and hence having multiple ways to emit > the crash information before running kdump kernel is a life-saver. If it is done in kernel boot phase before pid 1 comes up then things should be good enough, specific for kvm/hyper-v guests the kdump kernel. > > > > > But I think to set crash_kexec_post_notifiers by default is still bad. > > Because of the way it is run today I presume? If there was some > safe/unsafe policy that should work right? I would think that the > safe ones that work properly all the time are: > > - HyperV CRASH_MSRs, > - KVM PVPANIC_[PANIC,CRASHLOAD] push button knob, > - pstore EFI variables > - Dumping in memory, > > And then some that depend on firmware version (aka BIOS, and vendor) are: > - ACPI ERST, > > And then the unsafe: > - s390, PowerPC (I don't actually know what they are but that > was Dave's primary motivator). As I said we also got reports of kdump kernel hang with Hyper-V with the crash_kexec_post_notifiers enabled. EFI pstore also depends on efi runtime that is in firmware, also we can not ensure it works well after a panic happened. Ditto for other pstore backends we do not prefer to do it before kdump. But as I said I'm not saying they are not useful, people can use them by their choose. As for the virtual machine panic events maybe it is ok to add some other hooks instead of the notifiers. But frankly I still feel it is better to do it in kdump kernel boot path since kdump works well for virt from our experience. > > > > > > > > > > > > > > > > >> Further like I have mentioned everytime something like this has come up > > > >> a call on the kexec on panic code path should be a direct call (That > > > >> can > > > >> be audited) not something hidden in a notifier call chain (which can > > > >> not). > > > >> > > > > > > We btw already have a direct call from panic() to kmsg_dump() which is > > > indirectly controlled by crash_kexec_post_notifiers, and it would also be > > > preferable to be able to call it before kdump as well. > > > > Right, that is the same thing we are talking about. > > > > Thanks > > Dave > > > Thanks Dave
Re: [PATCH 1/5] arm64: spe: Allow new bits in SPE filter register
Hi Andre, On Tue, Sep 22, 2020 at 11:12:21AM +0100, Andre Przywara wrote: > The ARMv8.3-SPE extension adds some new bits for the event filter. > > Remove bits 11, 17 and 18 from the RES0 mask, so they can be used > correctly. > > Signed-off-by: Andre Przywara > --- > arch/arm64/include/asm/sysreg.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h > index 554a7e8ecb07..efca4ee28671 100644 > --- a/arch/arm64/include/asm/sysreg.h > +++ b/arch/arm64/include/asm/sysreg.h > @@ -281,7 +281,7 @@ > #define SYS_PMSFCR_EL1_ST_SHIFT 18 > > #define SYS_PMSEVFR_EL1 sys_reg(3, 0, 9, 9, 5) > -#define SYS_PMSEVFR_EL1_RES0 0x00ff0f55UL > +#define SYS_PMSEVFR_EL1_RES0 0x00f90755UL This patch is duplicate with Wei Li's patch [1]. You could see there have some discussion and Will gave suggestions [2] for the patch, this would be a good start point to continue this work. Thanks, Leo [1] https://www.spinics.net/lists/arm-kernel/msg825364.html [2] https://www.spinics.net/lists/arm-kernel/msg835733.html > > #define SYS_PMSLATFR_EL1 sys_reg(3, 0, 9, 9, 6) > #define SYS_PMSLATFR_EL1_MINLAT_SHIFT0 > -- > 2.17.1 >
Re: [PATCH 2/2] selftests/run_kselftest.sh: Make each test individually selectable
On Fri, Sep 25, 2020 at 04:45:27PM -0700, Kees Cook wrote: > Currently with run_kselftest.sh there is no way to choose which test > we could run. All the tests listed in kselftest-list.txt are all run > every time. This patch enhanced the run_kselftest.sh to make the test > collections (or tests) individually selectable. e.g.: > > $ ./run_kselftest.sh -c seccomp -t timers:posix_timers -t timers:nanosleep > > Additionally adds a way to list all known tests with "-l", usage > with "-h", and perform a dry run without running tests with "-n". This is better than my previous patch and we can modify run_kselftest.sh easily. The Documentation/dev-tools/kselftest.rst should also be update. Thanks Hangbin
[PATCH v5 3/5] counter: Add character device interface
This patch introduces a character device interface for the Counter subsystem. Device data is exposed through standard character device read operations. Device data is gathered when a Counter event is pushed by the respective Counter device driver. Configuration is handled via ioctl operations on the respective Counter character device node. A high-level view of how a count value is passed down from a counter driver is exemplified by the following. The driver callbacks are first registered to the Counter core component for use by the Counter userspace interface components: ++ | Counter device driver | ++ | Processes data from device | ++ | --- / driver callbacks / --- | V +--+ | Counter core | +--+ | Routes device driver | | callbacks to the | | userspace interfaces | +--+ | --- / driver callbacks / --- | +---+---+ | | V V ++ +-+ | Counter sysfs | | Counter chrdev | ++ +-+ | Translates to the | | Translates to the | | standard Counter | | standard Counter| | sysfs output | | character device| ++ +-+ Thereafter, data can be transferred directly between the Counter device driver and Counter userspace interface: -- / Counter device \ +--+ | Count register: 0x28 | +--+ | - / raw count data / - | V ++ | Counter device driver | ++ | Processes data from device | || | Type: u64 | | Value: 42 | ++ | -- / u64 / -- | +---+---+ | | V V ++ +-+ | Counter sysfs | | Counter chrdev | ++ +-+ | Translates to the | | Translates to the | | standard Counter | | standard Counter| | sysfs output | | character device| || |-| | Type: const char * | | Type: u64 | | Value: "42"| | Value: 42 | ++ +-+ | | --- --- / const char * // struct counter_event / --- --- | | | V | +---+ | | read | | +---+ | \ Count: 42 / |--- | V +--+ | `/sys/bus/counter/devices/counterX/countY/count` |
[PATCH v5 0/5] Introduce the Counter character device interface
Changes in v5: - Fixed typographical errors in documentation and comments - Updated flow charts in documentation for clarity - Moved uapi header to be part of the character device intro patch - Fix git squash mistake in 104-quad-8.c; remove redundant changes - Fix git merge mistake in 104-quad-8.c; fix locking race condition - Minor code cleanup for clarity; adjust whitespace/flow - Use put_device if device_add fails - Document sysfs structures - Rename "owner" symbols to "scope"; more apt name - Use resource-managed devm_* allocation functions - Rename *_free functions to *_remove; following common convention - Rename COUNTER_DATA* to COUNTER_COMP*; more obvious meaning - Rename various symbol and define names for clarity - Bring back static ops function; more secure to have static const - Rename counter_available union members to "enums" and "strs" - Implement COUNTER_EVENT* constants; event types are now standard - Implement atomic Counter watches swap; no more racy event config Over the past couple years we have noticed some shortcomings with the Counter sysfs interface. Although useful in the majority of situations, there are certain use-cases where interacting through sysfs attributes can become cumbersome and inefficient. A desire to support more advanced functionality such as timestamps, multi-axes positioning tables, and other such latency-sensitive applications, has motivated a reevaluation of the Counter subsystem. I believe a character device interface will be helpful for this more niche area of counter device use. To quell any concerns from the offset: this patchset makes no changes to the existing Counter sysfs userspace interface -- existing userspace applications will continue to work with no modifications necessary. I request that driver maintainers please test their applications to verify that this is true, and report any discrepancies if they arise. However, this patchset does contain a major reimplementation of the Counter subsystem core and driver API. A reimplementation was necessary in order to separate the sysfs code from the counter device drivers and internalize it as a dedicated component of the core Counter subsystem module. A minor benefit from all of this is that the sysfs interface is now ensured a certain amount of consistency because the translation is performed outside of individual counter device drivers. Essentially, the reimplementation has enabled counter device drivers to pass and handle data as native C datatypes now rather than the sysfs strings from before. A high-level view of how a count value is passed down from a counter driver is exemplified by the following. The driver callbacks are first registered to the Counter core component for use by the Counter userspace interface components: ++ | Counter device driver | ++ | Processes data from device | ++ | --- / driver callbacks / --- | V +--+ | Counter core | +--+ | Routes device driver | | callbacks to the | | userspace interfaces | +--+ | --- / driver callbacks / --- | +---+---+ | | V V ++ +-+ | Counter sysfs | | Counter chrdev | ++ +-+ | Translates to the | | Translates to the | | standard Counter | | standard Counter| | sysfs output | | character device| ++ +-+ Thereafter, data can be transferred directly between the Counter device driver and Counter userspace interface: -- / Counter device \ +--+ | Count register: 0x28 | +--+ | - / raw count data / -
[PATCH v5 5/5] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8
The LSI/CSI LS7266R1 chip provides programmable output via the FLG pins. When interrupts are enabled on the ACCES 104-QUAD-8, they occur whenever FLG1 is active. Four functions are available for the FLG1 signal: Carry, Compare, Carry-Borrow, and Index. Carry: Interrupt generated on active low Carry signal. Carry signal toggles every time the respective channel's counter overflows. Compare: Interrupt generated on active low Compare signal. Compare signal toggles every time respective channel's preset register is equal to the respective channel's counter. Carry-Borrow: Interrupt generated on active low Carry signal and active low Borrow signal. Carry signal toggles every time the respective channel's counter overflows. Borrow signal toggles every time the respective channel's counter underflows. Index: Interrupt generated on active high Index signal. The irq_trigger Count extension is introduced to allow the selection of the desired IRQ trigger function per channel. The irq_trigger_enable Count extension is introduced to allow the enablement of interrupts for a respective channel. Interrupts push Counter events to event channel X, where 'X' is the respective channel whose FLG1 activated. This patch adds IRQ support for the ACCES 104-QUAD-8. The interrupt line numbers for the devices may be configured via the irq array module parameter. Reviewed-by: Syed Nayyar Waris Signed-off-by: William Breathitt Gray --- .../ABI/testing/sysfs-bus-counter-104-quad-8 | 32 ++ drivers/counter/104-quad-8.c | 303 ++ drivers/counter/Kconfig | 6 +- 3 files changed, 270 insertions(+), 71 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-counter-104-quad-8 b/Documentation/ABI/testing/sysfs-bus-counter-104-quad-8 index eac32180c40d..995bf08d365d 100644 --- a/Documentation/ABI/testing/sysfs-bus-counter-104-quad-8 +++ b/Documentation/ABI/testing/sysfs-bus-counter-104-quad-8 @@ -1,3 +1,35 @@ +What: /sys/bus/counter/devices/counterX/countY/irq_trigger +KernelVersion: 5.11 +Contact: linux-...@vger.kernel.org +Description: + IRQ trigger function for channel Y. Four trigger functions are + available: carry, compare, carry-borrow, and index. + + carry: + Interrupt generated on active low Carry signal. Carry + signal toggles every time channel Y counter overflows. + + compare: + Interrupt generated on active low Compare signal. + Compare signal toggles every time channel Y preset + register is equal to channel Y counter. + + carry-borrow: + Interrupt generated on active low Carry signal and + active low Borrow signal. Carry signal toggles every + time channel Y counter overflows. Borrow signal toggles + every time channel Y counter underflows. + + index: + Interrupt generated on active high Index signal. + +What: /sys/bus/counter/devices/counterX/countY/irq_trigger_enable +KernelVersion: 5.11 +Contact: linux-...@vger.kernel.org +Description: + Whether generation of interrupts is enabled for channel Y. Valid + attribute values are boolean. + What: /sys/bus/counter/devices/counterX/signalY/cable_fault KernelVersion: 5.7 Contact: linux-...@vger.kernel.org diff --git a/drivers/counter/104-quad-8.c b/drivers/counter/104-quad-8.c index 92a3809609e9..d0be75a539dd 100644 --- a/drivers/counter/104-quad-8.c +++ b/drivers/counter/104-quad-8.c @@ -13,23 +13,30 @@ #include #include #include +#include #include #include #include #include #include +#include #define QUAD8_EXTENT 32 static unsigned int base[max_num_isa_dev(QUAD8_EXTENT)]; static unsigned int num_quad8; -module_param_array(base, uint, _quad8, 0); +module_param_hw_array(base, uint, ioport, _quad8, 0); MODULE_PARM_DESC(base, "ACCES 104-QUAD-8 base addresses"); +static unsigned int irq[max_num_isa_dev(QUAD8_EXTENT)]; +module_param_hw_array(irq, uint, irq, NULL, 0); +MODULE_PARM_DESC(irq, "ACCES 104-QUAD-8 interrupt line numbers"); + #define QUAD8_NUM_COUNTERS 8 /** * struct quad8_iio - IIO device private data structure + * @lock: synchronization lock to prevent I/O race conditions * @counter: instance of the counter_device * @fck_prescaler: array of filter clock prescaler configurations * @preset:array of preset values @@ -38,13 +45,14 @@ MODULE_PARM_DESC(base, "ACCES 104-QUAD-8 base addresses"); * @quadrature_scale: array
[PATCH v5 4/5] docs: counter: Document character device interface
This patch adds high-level documentation about the Counter subsystem character device interface. Signed-off-by: William Breathitt Gray --- Documentation/ABI/testing/sysfs-bus-counter | 18 ++ Documentation/driver-api/generic-counter.rst | 228 ++ .../userspace-api/ioctl/ioctl-number.rst | 1 + 3 files changed, 206 insertions(+), 41 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-counter b/Documentation/ABI/testing/sysfs-bus-counter index 566bd99fe0a5..b7fdb14ae891 100644 --- a/Documentation/ABI/testing/sysfs-bus-counter +++ b/Documentation/ABI/testing/sysfs-bus-counter @@ -99,6 +99,24 @@ Description: Read-only attribute that indicates whether excessive noise is present at the channel Y counter inputs. +What: /sys/bus/counter/devices/counterX/countY/extensionZ_name +What: /sys/bus/counter/devices/counterX/extensionZ_name +What: /sys/bus/counter/devices/counterX/signalY/extensionZ_name +KernelVersion: 5.11 +Contact: linux-...@vger.kernel.org +Description: + Read-only attribute that indicates the component name of + Extension Z. + +What: /sys/bus/counter/devices/counterX/countY/extensionZ_width +What: /sys/bus/counter/devices/counterX/extensionZ_width +What: /sys/bus/counter/devices/counterX/signalY/extensionZ_width +KernelVersion: 5.11 +Contact: linux-...@vger.kernel.org +Description: + Read-only attribute that indicates the data width of value of + Extension Z. + What: /sys/bus/counter/devices/counterX/countY/function KernelVersion: 5.2 Contact: linux-...@vger.kernel.org diff --git a/Documentation/driver-api/generic-counter.rst b/Documentation/driver-api/generic-counter.rst index b842ddbbd8a0..6077bf162ac3 100644 --- a/Documentation/driver-api/generic-counter.rst +++ b/Documentation/driver-api/generic-counter.rst @@ -223,19 +223,6 @@ whether an input line is differential or single-ended) and instead focus on the core idea of what the data and process represent (e.g. position as interpreted from quadrature encoding data). -Userspace Interface -=== - -Several sysfs attributes are generated by the Generic Counter interface, -and reside under the /sys/bus/counter/devices/counterX directory, where -counterX refers to the respective counter device. Please see -Documentation/ABI/testing/sysfs-bus-counter for detailed -information on each Generic Counter interface sysfs attribute. - -Through these sysfs attributes, programs and scripts may interact with -the Generic Counter paradigm Counts, Signals, and Synapses of respective -counter devices. - Driver API == @@ -387,16 +374,16 @@ userspace interface components:: / driver callbacks / --- | -+---+ -| -V -++ -| Counter sysfs | -++ -| Translates to the | -| standard Counter | -| sysfs output | -++ ++---+---+ +| | +V V +++ +-+ +| Counter sysfs | | Counter chrdev | +++ +-+ +| Translates to the | | Translates to the | +| standard Counter | | standard Counter| +| sysfs output | | character device| +++ +-+ Thereafter, data can be transferred directly between the Counter device driver and Counter userspace interface:: @@ -427,23 +414,30 @@ driver and Counter userspace interface:: / u64 / -- | -+---+ -| -V -++ -| Counter sysfs | -++ -| Translates to the | -| standard Counter | -| sysfs output | -|| -| Type: const char * | -| Value: "42"| -++ -| - --- -/ const char * / ---- ++---+---+ +| | +V V +++ +-+ +| Counter sysfs | | Counter chrdev | +++ +-+ +| Translates to the |
[PATCH v5 2/5] docs: counter: Update to reflect sysfs internalization
The Counter subsystem architecture and driver implementations have
changed in order to handle Counter sysfs interactions in a more
consistent way. This patch updates the Generic Counter interface
documentation to reflect the changes.
Signed-off-by: William Breathitt Gray
---
Documentation/driver-api/generic-counter.rst | 242 ++-
1 file changed, 176 insertions(+), 66 deletions(-)
diff --git a/Documentation/driver-api/generic-counter.rst
b/Documentation/driver-api/generic-counter.rst
index b02c52cd69d6..b842ddbbd8a0 100644
--- a/Documentation/driver-api/generic-counter.rst
+++ b/Documentation/driver-api/generic-counter.rst
@@ -250,8 +250,8 @@ for defining a counter device.
.. kernel-doc:: drivers/counter/counter.c
:export:
-Implementation
-==
+Driver Implementation
+=
To support a counter device, a driver must first allocate the available
Counter Signals via counter_signal structures. These Signals should
@@ -267,25 +267,59 @@ respective counter_count structure. These counter_count
structures are
set to the counts array member of an allocated counter_device structure
before the Counter is registered to the system.
-Driver callbacks should be provided to the counter_device structure via
-a constant counter_ops structure in order to communicate with the
-device: to read and write various Signals and Counts, and to set and get
-the "action mode" and "function mode" for various Synapses and Counts
-respectively.
+Driver callbacks must be provided to the counter_device structure in
+order to communicate with the device: to read and write various Signals
+and Counts, and to set and get the "action mode" and "function mode" for
+various Synapses and Counts respectively.
A defined counter_device structure may be registered to the system by
passing it to the counter_register function, and unregistered by passing
it to the counter_unregister function. Similarly, the
-devm_counter_register and devm_counter_unregister functions may be used
-if device memory-managed registration is desired.
-
-Extension sysfs attributes can be created for auxiliary functionality
-and data by passing in defined counter_device_ext, counter_count_ext,
-and counter_signal_ext structures. In these cases, the
-counter_device_ext structure is used for global/miscellaneous exposure
-and configuration of the respective Counter device, while the
-counter_count_ext and counter_signal_ext structures allow for auxiliary
-exposure and configuration of a specific Count or Signal respectively.
+devm_counter_register function may be used if device memory-managed
+registration is desired.
+
+The struct counter_comp structure is used to define counter extensions
+for Signals, Synapses, and Counts.
+
+The "type" member specifies the type of high-level data (e.g. BOOL,
+COUNT_DIRECTION, etc.) handled by this extension. The "`*_read`" and
+"`*_write`" members can then be set by the counter device driver with
+callbacks to handle that data using native C data types (i.e. u8, u64,
+etc.).
+
+Convenience macros such as `COUNTER_COMP_COUNT_U64` are provided for use
+by driver authors. In particular, driver authors are expected to use
+the provided macros for standard Counter subsystem attributes in order
+to maintain a consistent interface for userspace. For example, a counter
+device driver may define several standard attributes like so::
+
+struct counter_comp count_ext[] = {
+COUNTER_COMP_DIRECTION(count_direction_read),
+COUNTER_COMP_ENABLE(count_enable_read, count_enable_write),
+COUNTER_COMP_CEILING(count_ceiling_read, count_ceiling_write),
+};
+
+This makes it simple to see, add, and modify the attributes that are
+supported by this driver ("direction", "enable", and "ceiling") and to
+maintain this code without getting lost in a web of struct braces.
+
+Callbacks must match the function type expected for the respective
+component or extension. These function types are defined in the struct
+counter_comp structure as the "`*_read`" and "`*_write`" union members.
+
+The corresponding callback prototypes for the extensions mentioned in
+the previous example above would be::
+
+int count_direction_read(struct counter_device *counter,
+ struct counter_count *count, u8 *direction);
+int count_enable_read(struct counter_device *counter,
+ struct counter_count *count, u8 *enable);
+int count_enable_write(struct counter_device *counter,
+ struct counter_count *count, u8 enable);
+int count_ceiling_read(struct counter_device *counter,
+ struct counter_count *count, u64 *ceiling);
+int count_ceiling_write(struct counter_device *counter,
+struct counter_count *count, u64 ceiling);
Determining the type of extension to create is a matter
Re: [PATCH] Documentation: Chinese translation of Documentation/arm64/perf.rst
在 2020/9/27 上午4:15, Jonathan Corbet 写道: > On Sat, 26 Sep 2020 22:35:51 +0800 > Alex Shi wrote: > >> Why your patch repeatly has encoding issue which fails on 'git am' >> Could you like to check the problem before send out? >> Could you please fix your editor issue by >> Documentation/process/email-clients.rst >> or send patch by git send-email. >> >> And please don't waste other time on meaningless issue again! > > The way to be sure you have solved this kind of problem is to first email > the patch to you, then be sure that what you receive can be applied. > Please get to the point where that works, then I'll be glad to apply your > translations. > CC Qing, There are few documents of how to join the community development https://www.kernel.org/doc/html/latest/translations/zh_CN/index.html Shortly, sth could be summaried as following in pariticular documents. 1, setup well your email client, to be sure all your patch from your email system could be applied by 'git am'. Current email client often cover the endcoding issue but git will failed on that. 2, 'make help' show couple of doc related options: htmldocs- HTML latexdocs - LaTeX pdfdocs - PDF epubdocs- EPUB xmldocs - XML linkcheckdocs - check for broken external links (will connect to external hosts) refcheckdocs- check for references to non-existing files under Documentation cleandocs - clean all generated files Do 'make linkcheckdocs/refcheckdocs/htmldocs', and check if the result works as expected, of course it would be better if you can make/check all type docs, but at least finish above 3 kinds of checks. 3, As to related docs, sending them as a couple series could help people retrieve easy. looking forward for your new translation docs. Thanks Alex > Thanks, > > jon >
drivers/staging/media/allegro-dvt/allegro-core.c:3142:34: warning: unused variable 'allegro_dt_ids'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: a1bffa48745afbb54cb4f873bba783b2ae8be042
commit: a19f228b8dd9a67e8de4ebd4eac8a4c94ec39d1a media: Kconfig: not all V4L2
platform drivers are for camera
date: 6 months ago
config: x86_64-randconfig-a001-20200927 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
a83eb048cb9a75da7a07a9d5318bbdbf54885c87)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a19f228b8dd9a67e8de4ebd4eac8a4c94ec39d1a
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout a19f228b8dd9a67e8de4ebd4eac8a4c94ec39d1a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/staging/media/allegro-dvt/allegro-core.c:3142:34: warning: unused
>> variable 'allegro_dt_ids' [-Wunused-const-variable]
static const struct of_device_id allegro_dt_ids[] = {
^
1 warning generated.
vim +/allegro_dt_ids +3142 drivers/staging/media/allegro-dvt/allegro-core.c
f20387dfd065693 Michael Tretter 2019-05-28 3141
f20387dfd065693 Michael Tretter 2019-05-28 @3142 static const struct
of_device_id allegro_dt_ids[] = {
f20387dfd065693 Michael Tretter 2019-05-28 3143{ .compatible =
"allegro,al5e-1.1" },
f20387dfd065693 Michael Tretter 2019-05-28 3144{ /* sentinel */ }
f20387dfd065693 Michael Tretter 2019-05-28 3145 };
f20387dfd065693 Michael Tretter 2019-05-28 3146
:: The code at line 3142 was first introduced by commit
:: f20387dfd065693ba7ea2788a2f893bf653c9cb8 media: allegro: add Allegro DVT
video IP core driver
:: TO: Michael Tretter
:: CC: Mauro Carvalho Chehab
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH] ocfs2: fix potential soft lockup during fstrim
When we discard unused blocks on a mounted ocfs2 filesystem, fstrim
handles each block goup with locking/unlocking global bitmap meta-file
repeatedly. we should let fstrim thread take a break(if need) between
unlock and lock, this will avoid the potential soft lockup problem,
and also gives the upper applications more IO opportunities, these
applications are not blocked for too long at writing files.
Signed-off-by: Gang He
---
fs/ocfs2/alloc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index 4c1b90442d6f..2cf9321919b5 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -7654,8 +7654,10 @@ int ocfs2_trim_mainbm(struct super_block *sb, struct
fstrim_range *range)
* main_bm related locks for avoiding the current IO starve, then go to
* trim the next group
*/
- if (ret >= 0 && group <= last_group)
+ if (ret >= 0 && group <= last_group) {
+ cond_resched();
goto next_group;
+ }
out:
range->len = trimmed * sb->s_blocksize;
return ret;
--
2.21.0
RE: [PATCH 1/5] PCI: define a function to check and wait till port finish DPC handling
Andy,
About the header file, yes, to keep the order.
The function was already defined with #ifdef CONFIG_PCIE_DPC.
As to ' readx_poll_timeout()' if there is generic one, I would like to use
it.
Seems there is no yet ?
Thanks,
Ethan
-Original Message-
From: Andy Shevchenko
Sent: Friday, September 25, 2020 8:25 PM
To: Zhao, Haifeng
Cc: bhelg...@google.com; ooh...@gmail.com; rus...@russell.cc; lu...@wunner.de;
stuart.w.ha...@gmail.com; mr.nuke...@gmail.com;
mika.westerb...@linux.intel.com; linux-...@vger.kernel.org;
linux-kernel@vger.kernel.org; Jia, Pei P
Subject: Re: [PATCH 1/5] PCI: define a function to check and wait till port
finish DPC handling
On Thu, Sep 24, 2020 at 10:34:19PM -0400, Ethan Zhao wrote:
> Once root port DPC capability is enabled and triggered, at the
> beginning of DPC is triggered, the DPC status bits are set by hardware
> and then sends DPC/DLLSC/PDC interrupts to OS DPC and pciehp drivers,
> it will take the port and software DPC interrupt handler 10ms to 50ms
> (test data on ICX platform & stable 5.9-rc6) to complete the DPC
> containment procedure till the DPC status is cleared at the end of the DPC
> interrupt handler.
>
> We use this function to check if the root port is in DPC handling
> status and wait till the hardware and software completed the procedure.
> #include
> #include
> #include
> +#include
Keep it sorted?
> #include
...
> +#ifdef CONFIG_PCIE_DPC
> +static inline bool pci_wait_port_outdpc(struct pci_dev *pdev) {
> + u16 cap = pdev->dpc_cap, status;
> + u16 loop = 0;
> +
> + if (!cap) {
> + pci_WARN_ONCE(pdev, !cap, "No DPC capability initiated\n");
But why? Is this feature mandatory to have? Then the same question about
ifdeffery, otherwise it's pretty normal to not have a feature, right?
> + return false;
> + }
> + pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, );
> + pci_dbg(pdev, "DPC status %x, cap %x\n", status, cap);
> + while (status & PCI_EXP_DPC_STATUS_TRIGGER && loop < 100) {
> + msleep(10);
> + loop++;
> + pci_read_config_word(pdev, cap + PCI_EXP_DPC_STATUS, );
> + }
Can we have rather something like readx_poll_timeout() for PCI and use them
here?
> + if (!(status & PCI_EXP_DPC_STATUS_TRIGGER)) {
> + pci_dbg(pdev, "Out of DPC status %x, time cost %d ms\n",
> status, loop*10);
> + return true;
> + }
> + pci_dbg(pdev, "Timeout to wait port out of DPC status\n");
> + return false;
> +}
> +#else
> +static inline bool pci_wait_port_outdpc(struct pci_dev *pdev) {
> + return true;
> +}
> +#endif
> #endif /* LINUX_PCI_H */
> --
> 2.18.4
>
--
With Best Regards,
Andy Shevchenko
Re: [PATCHv5 kselftest next] selftests/run_kselftest.sh: make each test individually selectable
On Fri, Sep 25, 2020 at 02:16:14PM -0700, Kees Cook wrote:
> On Fri, Sep 25, 2020 at 01:51:53PM +0530, Naresh Kamboju wrote:
> > On Mon, 14 Sep 2020 at 07:53, Hangbin Liu wrote:
> > >
> > > Currently, after generating run_kselftest.sh, there is no way to choose
> > > which test we could run. All the tests are listed together and we have
> > > to run all every time. This patch enhanced the run_kselftest.sh to make
> > > the tests individually selectable. e.g.
> > >
> > > $ ./run_kselftest.sh -t "bpf size timers"
> >
> > My test run break on linux next
> >
> > ./run_kselftest.sh: line 1331: syntax error near unexpected token `)'
> > ./run_kselftest.sh: line 1331: `-e -s | --summary )
> > logfile=$BASE_DIR/output.log; cat /dev/null > $logfile; shift ;;'
>
> Yes, please revert this patch. The resulting script is completely
> trashed:
>
> BASE_DIR=$(realpath $(dirname $0))
> . ./kselftest/runner.sh
> TESTS="seccomp"
>
> run_seccomp()
> {
> -e [ -w /dev/kmsg ] && echo "kselftest: Running tests in seccomp" >>
> /dev/kmsg
> -e cd seccomp
> -en run_many
> \
> -ne "seccomp_bpf"
> \
> -ne "seccomp_benchmark"
>
> -e cd $ROOT
> }
I'm really sorry to make this trouble. And I'm OK to revert the patch.
I just a little wondering how do you generate this script.
I tested with 'make -C tools/testing/selftests gen_tar FORMAT=.xz' before
post the patch and all looks good to me.
```
run_seccomp()
{
[ -w /dev/kmsg ] && echo "kselftest: Running tests in seccomp" >>
/dev/kmsg
cd seccomp
run_many\
"seccomp_bpf" \
"seccomp_benchmark"
cd $ROOT
}
```
Thanks
Hangbin
RE: [PATCH] KVM: x86: emulate wait-for-SIPI and SIPI-VMExit
> Subject: RE: [PATCH] KVM: x86: emulate wait-for-SIPI and SIPI-VMExit > > > Again, this looks good but it needs testcases. > > Yes, the unit test development is WIP. > Hi, Paolo I have sent out the unit test patch. https://patchwork.kernel.org/patch/11799305/ Could you help review it? Thanks a lot. Best Regard Yadong
RE: [PATCH 2/5] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC
Andy, About the ICX code name, I will align it to public documentation in next version. ' non-relevant information' about the crash, I'm not sure what's not relevant. Thanks, Ethan -Original Message- From: Andy Shevchenko Sent: Friday, September 25, 2020 8:33 PM To: Zhao, Haifeng Cc: bhelg...@google.com; ooh...@gmail.com; rus...@russell.cc; lu...@wunner.de; stuart.w.ha...@gmail.com; mr.nuke...@gmail.com; mika.westerb...@linux.intel.com; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Jia, Pei P Subject: Re: [PATCH 2/5] PCI: pciehp: check and wait port status out of DPC before handling DLLSC and PDC On Thu, Sep 24, 2020 at 10:34:20PM -0400, Ethan Zhao wrote: > When root port has DPC capability and it is enabled, then triggered by > errors, DPC DLLSC and PDC interrupts will be sent to DPC driver, pciehp > driver at the same time. > That will cause following result: > > 1. Link and device are recovered by hardware DPC and software DPC driver, > device >isn't removed, but the pciehp might treat it as devce was hot removed. > > 2. Race condition happens between pciehp_unconfigure_device() called by >pciehp_ist() in pciehp driver and pci_do_recovery() called by dpc_handler > in >DPC driver. no luck, there is no lock to protect > pci_stop_and_remove_bus_device() >against pci_walk_bus(), they hold different semaphore and mutex, >pci_stop_and_remove_bus_device holds pci_rescan_remove_lock, and > pci_walk_bus() >holds pci_bus_sem. > > This race condition is not purely code analysis, it could be triggered > by following command series: > > # setpci -s 64:02.0 0x196.w=000a // 64:02.0 is rootport has DPC capability > # setpci -s 65:00.0 0x04.w=0544// 65:00.0 is NVMe SSD populated in > above port > # mount /dev/nvme0n1p1 nvme > > One shot will cause system panic and null pointer reference happened. > (tested on stable 5.8 & ICX platform) What is ICX (yes, I know, but you are writing this for wider audience)? >Buffer I/O error on dev nvme0n1p1, logical block 468843328, async page read >BUG: kernel NULL pointer dereference, address: 0050 >#PF: supervisor read access in kernel mode >#PF: error_code(0x) - not-present page Please, try to remove non-relevant information from the crashes. >Oops: [#1] SMP NOPTI >CPU: 12 PID: 513 Comm: irq/124-pcie-dp Not tainted 5.8.0-0.0.7.el8.x86_64+ > #1 >Hardware name: Intel Corporation Wil.200x0x0xxx 0x/0x/20x0 >RIP: 0010:report_error_detected.cold.4+0x7d/0xe6 >Code: b6 d0 e8 e8 fe 11 00 e8 16 c5 fb ff be 06 00 00 00 48 89 df e8 d3 65 > ff >ff b8 06 00 00 00 e9 75 fc ff ff 48 8b 43 68 45 31 c9 <48> 8b 50 50 48 83 > 3a 00 >41 0f 94 c1 45 31 c0 48 85 d2 41 0f 94 c0 >RSP: 0018:ff8e06cf8762fda8 EFLAGS: 00010246 >RAX: RBX: ff4e3eaacf42a000 RCX: ff4e3eb31f223c01 >RDX: ff4e3eaacf42a140 RSI: ff4e3eb31f223c00 RDI: ff4e3eaacf42a138 >RBP: ff8e06cf8762fdd0 R08: 00bf R09: >R10: 00eb8ebeab53 R11: 93453258 R12: 0002 >R13: ff4e3eaacf42a130 R14: ff8e06cf8762fe2c R15: ff4e3eab44733828 >FS: () GS:ff4e3eab1fd0() > knlGS: >CS: 0010 DS: ES: CR0: 80050033 >CR2: 0050 CR3: 000f8f80a004 CR4: 00761ee0 >DR0: DR1: DR2: >DR3: DR6: fffe0ff0 DR7: 0400 >PKRU: 5554 >Call Trace: >? report_normal_detected+0x20/0x20 >report_frozen_detected+0x16/0x20 >pci_walk_bus+0x75/0x90 >? dpc_irq+0x90/0x90 >pcie_do_recovery+0x157/0x201 >? irq_finalize_oneshot.part.47+0xe0/0xe0 >dpc_handler+0x29/0x40 >irq_thread_fn+0x24/0x60 >irq_thread+0xea/0x170 >? irq_forced_thread_fn+0x80/0x80 >? irq_thread_check_affinity+0xf0/0xf0 >kthread+0x124/0x140 >? kthread_park+0x90/0x90 >ret_from_fork+0x1f/0x30 >Modules linked in: nft_fib_inet. >CR2: 0050 > > With this patch, the handling flow of DPC containment and hotplug is > partly ordered and serialized, let hardware DPC do the controller > reset etc recovery action first, then DPC driver handling the > call-back from device drivers, clear the DPC status, at the end, pciehp > driver handles the DLLSC and PDC etc. > This patch closes the race conditon partly. Not sure if Bjorn require to get rid of "this patch" from commit messages... In any case it's written in documentation. -- With Best Regards, Andy Shevchenko
sound/soc/meson/t9015.c:315:34: warning: unused variable 't9015_ids'
Hi Jerome,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: a1bffa48745afbb54cb4f873bba783b2ae8be042
commit: 33901f5b9b16d212ee58865e9e8e80fc813f12da ASoC: meson: add t9015
internal DAC driver
date: 7 months ago
config: x86_64-randconfig-r033-20200927 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project
a83eb048cb9a75da7a07a9d5318bbdbf54885c87)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
#
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=33901f5b9b16d212ee58865e9e8e80fc813f12da
git remote add linus
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git fetch --no-tags linus master
git checkout 33901f5b9b16d212ee58865e9e8e80fc813f12da
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> sound/soc/meson/t9015.c:315:34: warning: unused variable 't9015_ids'
>> [-Wunused-const-variable]
static const struct of_device_id t9015_ids[] = {
^
1 warning generated.
vim +/t9015_ids +315 sound/soc/meson/t9015.c
314
> 315 static const struct of_device_id t9015_ids[] = {
316 { .compatible = "amlogic,t9015", },
317 { }
318 };
319 MODULE_DEVICE_TABLE(of, t9015_ids);
320
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v10 12/16] s390/zcrypt: Notify driver on config changed and scan complete callbacks
On Fri, 21 Aug 2020 15:56:12 -0400
Tony Krowiak wrote:
> This patch intruduces an extension to the ap bus to notify drivers
> on crypto config changed and bus scan complete events.
> Two new callbacks are introduced for ap_drivers:
>
> void (*on_config_changed)(struct ap_config_info *new_config_info,
> struct ap_config_info *old_config_info);
> void (*on_scan_complete)(struct ap_config_info *new_config_info,
> struct ap_config_info *old_config_info);
>
> Both callbacks are optional. Both callbacks are only triggered
> when QCI information is available (facility bit 12):
>
> * The on_config_changed callback is invoked at the start of the AP bus scan
> function when it determines that the host AP configuration information
> has changed since the previous scan. This is done by storing
> an old and current QCI info struct and comparing them. If there is any
> difference, the callback is invoked.
>
> Note that when the AP bus scan detects that AP adapters or domains have
> been removed from the host's AP configuration, it will remove the
> associated devices from the AP bus subsystem's device model. This
> callback gives the device driver a chance to respond to the removal
> of the AP devices in bulk rather than one at a time as its remove
> callback is invoked. It will also allow the device driver to do any
> any cleanup prior to giving control back to the bus piecemeal. This is
> particularly important for the vfio_ap driver because there may be
> guests using the queues at the time.
>
> * The on_scan_complete callback is invoked after the ap bus scan is
> complete if the host AP configuration data has changed.
>
> Note that when the AP bus scan detects that adapters or domains have
> been added to the host's configuration, it will create new devices in
> the AP bus subsystem's device model. This callback also allows the driver
> to process all of the new devices in bulk.
>
> Please note that changes to the apmask and aqmask do not trigger
> these two callbacks since the bus scan function is not invoked by changes
> to those masks.
>
> Signed-off-by: Harald Freudenberger
> Signed-off-by: Tony Krowiak
> ---
> drivers/s390/crypto/ap_bus.c | 85 +++-
> drivers/s390/crypto/ap_bus.h | 12 +
> 2 files changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
> index db27bd931308..cbf4c4d2e573 100644
> --- a/drivers/s390/crypto/ap_bus.c
> +++ b/drivers/s390/crypto/ap_bus.c
> @@ -73,8 +73,10 @@ struct ap_perms ap_perms;
> EXPORT_SYMBOL(ap_perms);
> DEFINE_MUTEX(ap_perms_mutex);
> EXPORT_SYMBOL(ap_perms_mutex);
> +DEFINE_MUTEX(ap_config_lock);
>
> static struct ap_config_info *ap_qci_info;
> +static struct ap_config_info *ap_qci_info_old;
>
> /*
> * AP bus related debug feature things.
> @@ -1412,6 +1414,52 @@ static int __match_queue_device_with_queue_id(struct
> device *dev, const void *da
> && AP_QID_QUEUE(to_ap_queue(dev)->qid) == (int)(long) data;
> }
>
> +/* Helper function for notify_config_changed */
> +static int __drv_notify_config_changed(struct device_driver *drv, void *data)
> +{
> + struct ap_driver *ap_drv = to_ap_drv(drv);
> +
> + if (try_module_get(drv->owner)) {
> + if (ap_drv->on_config_changed)
> + ap_drv->on_config_changed(ap_qci_info,
> + ap_qci_info_old);
> + module_put(drv->owner);
> + }
> +
> + return 0;
> +}
> +
> +/* Notify all drivers about an qci config change */
> +static inline void notify_config_changed(void)
> +{
> + bus_for_each_drv(_bus_type, NULL, NULL,
> + __drv_notify_config_changed);
> +}
> +
> +/* Helper function for notify_scan_complete */
> +static int __drv_notify_scan_complete(struct device_driver *drv, void *data)
> +{
> + struct ap_driver *ap_drv = to_ap_drv(drv);
> +
> + if (try_module_get(drv->owner)) {
> + if (ap_drv->on_scan_complete)
> + ap_drv->on_scan_complete(ap_qci_info,
> + ap_qci_info_old);
> + module_put(drv->owner);
> + }
> +
> + return 0;
> +}
> +
> +/* Notify all drivers about bus scan complete */
> +static inline void notify_scan_complete(void)
> +{
> + bus_for_each_drv(_bus_type, NULL, NULL,
> + __drv_notify_scan_complete);
> +}
> +
> +
> +
Too many blank lines?
> /*
> * Helper function for ap_scan_bus().
> * Does the scan bus job for the given adapter id.
> @@ -1565,15 +1613,44 @@ static void _ap_scan_bus_adapter(int id)
> put_device(>ap_dev.device);
> }
>
> +static int ap_config_changed(void)
> +{
> + int cfg_chg = 0;
> +
> + if (ap_qci_info) {
> + if (!ap_qci_info_old) {
> + ap_qci_info_old =
RE: [PATCH 4/5] PCI: only return true when dev io state is really changed
Yes, better !
-Original Message-
From: Andy Shevchenko
Sent: Friday, September 25, 2020 8:38 PM
To: Zhao, Haifeng
Cc: bhelg...@google.com; ooh...@gmail.com; rus...@russell.cc; lu...@wunner.de;
stuart.w.ha...@gmail.com; mr.nuke...@gmail.com;
mika.westerb...@linux.intel.com; linux-...@vger.kernel.org;
linux-kernel@vger.kernel.org; Jia, Pei P
Subject: Re: [PATCH 4/5] PCI: only return true when dev io state is really
changed
On Thu, Sep 24, 2020 at 10:34:22PM -0400, Ethan Zhao wrote:
> When uncorrectable error happens, AER driver and DPC driver interrupt
> handlers likely call
>pcie_do_recovery()->pci_walk_bus()->report_frozen_detected() with
> pci_channel_io_frozen the same time.
Call chains are better to read if they split like
foo() ->
bar() ->
baz()
>If pci_dev_set_io_state() return true even if the original state is
> pci_channel_io_frozen, that will cause AER or DPC handler re-enter the
> error detecting and recovery procedure one after another.
>The result is the recovery flow mixed between AER and DPC.
> So simplify the pci_dev_set_io_state() function to only return true
> when dev->error_state is changed.
...
> + if (dev->error_state != new) {
> dev->error_state = new;
> + changed = true;
> + }
> return changed;
Perhaps
if (dev->error_state == new)
return changed;
dev->error_state = new;
return true;
?
--
With Best Regards,
Andy Shevchenko
[rcu:urezki-pcount.2020.09.26a 5/17] include/linux/pagemap.h:181:2: error: called object type 'void' is not a function or function pointer
tree: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git urezki-pcount.2020.09.26a head: e9bed2a1239b017d78cec5de66adce0560f6d077 commit: 2fa3b3dd18ef5ef28a9dd40f6711211c62ac929b [5/17] mm/pagemap: Cleanup PREEMPT_COUNT leftovers config: x86_64-randconfig-a002-20200927 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project a83eb048cb9a75da7a07a9d5318bbdbf54885c87) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?id=2fa3b3dd18ef5ef28a9dd40f6711211c62ac929b git remote add rcu https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git git fetch --no-tags rcu urezki-pcount.2020.09.26a git checkout 2fa3b3dd18ef5ef28a9dd40f6711211c62ac929b # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): In file included from arch/x86/kernel/asm-offsets.c:13: In file included from include/linux/suspend.h:5: In file included from include/linux/swap.h:9: In file included from include/linux/memcontrol.h:22: In file included from include/linux/writeback.h:14: In file included from include/linux/blk-cgroup.h:23: In file included from include/linux/blkdev.h:13: >> include/linux/pagemap.h:181:2: error: called object type 'void' is not a >> function or function pointer VM_BUG_ON_PAGE(page_count(page) == 0, page); ^ include/linux/mmdebug.h:46:36: note: expanded from macro 'VM_BUG_ON_PAGE' #define VM_BUG_ON_PAGE(cond, page) VM_BUG_ON(cond) ^ include/linux/mmdebug.h:45:25: note: expanded from macro 'VM_BUG_ON' #define VM_BUG_ON(cond) BUILD_BUG_ON_INVALID(cond) ^ include/linux/build_bug.h:30:33: note: expanded from macro 'BUILD_BUG_ON_INVALID' #define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e ^ 1 error generated. make[2]: *** [scripts/Makefile.build:117: arch/x86/kernel/asm-offsets.s] Error 1 make[2]: Target '__build' not remade because of errors. make[1]: *** [Makefile:1198: prepare0] Error 2 make[1]: Target 'prepare' not remade because of errors. make: *** [Makefile:185: __sub-make] Error 2 make: Target 'prepare' not remade because of errors. vim +/void +181 include/linux/pagemap.h ^1da177e4c3f41 Linus Torvalds 2005-04-16 123 e286781d5f2e9c Nick Piggin2008-07-25 124 /* e286781d5f2e9c Nick Piggin2008-07-25 125 * speculatively take a reference to a page. 0139aa7b7fa12c Joonsoo Kim2016-05-19 126 * If the page is free (_refcount == 0), then _refcount is untouched, and 0 0139aa7b7fa12c Joonsoo Kim2016-05-19 127 * is returned. Otherwise, _refcount is incremented by 1 and 1 is returned. e286781d5f2e9c Nick Piggin2008-07-25 128 * e286781d5f2e9c Nick Piggin2008-07-25 129 * This function must be called inside the same rcu_read_lock() section as has e286781d5f2e9c Nick Piggin2008-07-25 130 * been used to lookup the page in the pagecache radix-tree (or page table): 0139aa7b7fa12c Joonsoo Kim2016-05-19 131 * this allows allocators to use a synchronize_rcu() to stabilize _refcount. e286781d5f2e9c Nick Piggin2008-07-25 132 * e286781d5f2e9c Nick Piggin2008-07-25 133 * Unless an RCU grace period has passed, the count of all pages coming out e286781d5f2e9c Nick Piggin2008-07-25 134 * of the allocator must be considered unstable. page_count may return higher e286781d5f2e9c Nick Piggin2008-07-25 135 * than expected, and put_page must be able to do the right thing when the e286781d5f2e9c Nick Piggin2008-07-25 136 * page has been finished with, no matter what it is subsequently allocated e286781d5f2e9c Nick Piggin2008-07-25 137 * for (because put_page is what is used here to drop an invalid speculative e286781d5f2e9c Nick Piggin2008-07-25 138 * reference). e286781d5f2e9c Nick Piggin2008-07-25 139 * e286781d5f2e9c Nick Piggin2008-07-25 140 * This is the interesting part of the lockless pagecache (and lockless e286781d5f2e9c Nick Piggin2008-07-25 141 * get_user_pages) locking protocol, where the lookup-side (eg. find_get_page) e286781d5f2e9c Nick Piggin2008-07-25 142 * has the following pattern: e286781d5f2e9c Nick
Re: WARNING in hrtimer_forward
syzbot has found a reproducer for the following issue on: HEAD commit:ba5f4cfe bpf: Add comment to document BTF type PTR_TO_BTF_.. git tree: bpf-next console output: https://syzkaller.appspot.com/x/log.txt?x=13f316e590 kernel config: https://syzkaller.appspot.com/x/.config?x=d44e1360b76d34dc dashboard link: https://syzkaller.appspot.com/bug?extid=ca740b95a16399ceb9a5 compiler: gcc (GCC) 10.1.0-syz 20200507 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1148fe4b90 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12f5218d90 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+ca740b95a16399ceb...@syzkaller.appspotmail.com [ cut here ] WARNING: CPU: 0 PID: 6901 at kernel/time/hrtimer.c:932 hrtimer_forward+0x1e3/0x260 kernel/time/hrtimer.c:932 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 6901 Comm: kworker/u4:1 Not tainted 5.9.0-rc6-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: phy4 ieee80211_iface_work Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x198/0x1fd lib/dump_stack.c:118 panic+0x382/0x7fb kernel/panic.c:231 __warn.cold+0x20/0x4b kernel/panic.c:600 report_bug+0x1bd/0x210 lib/bug.c:198 handle_bug+0x38/0x90 arch/x86/kernel/traps.c:234 exc_invalid_op+0x14/0x40 arch/x86/kernel/traps.c:254 asm_exc_invalid_op+0x12/0x20 arch/x86/include/asm/idtentry.h:536 RIP: 0010:hrtimer_forward+0x1e3/0x260 kernel/time/hrtimer.c:932 Code: e5 4d 0f 4e ec e8 ad 24 10 00 4c 89 6b 20 e8 a4 24 10 00 4c 89 f0 48 83 c4 18 5b 5d 41 5c 41 5d 41 5e 41 5f c3 e8 8d 24 10 00 <0f> 0b 45 31 f6 eb dd e8 81 24 10 00 4c 89 e0 48 8b 3c 24 48 99 48 RSP: 0018:c9007d90 EFLAGS: 00010246 RAX: RBX: 88808ded4b78 RCX: 81666168 RDX: 8880942f0200 RSI: 816662b3 RDI: 0001 RBP: 061a8000 R08: 0001 R09: 8880942f0b00 R10: R11: R12: R13: 00a6d77ff62e R14: 0001 R15: dc00 mac80211_hwsim_beacon+0x159/0x1a0 drivers/net/wireless/mac80211_hwsim.c:1726 __run_hrtimer kernel/time/hrtimer.c:1524 [inline] __hrtimer_run_queues+0x6a9/0xfc0 kernel/time/hrtimer.c:1588 hrtimer_run_softirq+0x17b/0x360 kernel/time/hrtimer.c:1605 __do_softirq+0x1f8/0xb23 kernel/softirq.c:298 asm_call_on_stack+0xf/0x20 arch/x86/entry/entry_64.S:706 __run_on_irqstack arch/x86/include/asm/irq_stack.h:22 [inline] run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:48 [inline] do_softirq_own_stack+0x9d/0xd0 arch/x86/kernel/irq_64.c:77 invoke_softirq kernel/softirq.c:393 [inline] __irq_exit_rcu kernel/softirq.c:423 [inline] irq_exit_rcu+0x235/0x280 kernel/softirq.c:435 sysvec_apic_timer_interrupt+0x51/0xf0 arch/x86/kernel/apic/apic.c:1091 asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:581 RIP: 0010:arch_local_irq_restore arch/x86/include/asm/paravirt.h:770 [inline] RIP: 0010:lock_acquire+0x27b/0xaf0 kernel/locking/lockdep.c:5032 Code: ba 00 00 00 00 00 fc ff df 48 c1 e8 03 80 3c 10 00 0f 85 1d 07 00 00 48 83 3d d8 41 a0 08 00 0f 84 73 05 00 00 4c 89 ff 57 9d <0f> 1f 44 00 00 48 b8 00 00 00 00 00 fc ff df 48 03 44 24 08 48 c7 RSP: 0018:c9e37c18 EFLAGS: 0286 RAX: 113f8d7d RBX: 8880942f0200 RCX: 0001 RDX: dc00 RSI: 0008 RDI: 0286 RBP: c9e37da8 R08: R09: 8d108aa7 R10: fbfff1a21154 R11: R12: R13: R14: R15: 0286 process_one_work+0x8bb/0x1670 kernel/workqueue.c:2245 worker_thread+0x64c/0x1120 kernel/workqueue.c:2415 kthread+0x3b5/0x4a0 kernel/kthread.c:292 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 Kernel Offset: disabled Rebooting in 86400 seconds..
Re: [V2] riscv: fix pfn_to_virt err in do_page_fault().
On Fri, 18 Sep 2020 01:55:58 PDT (-0700), li...@allwinnertech.com wrote: The argument to pfn_to_virt() should be pfn not the value of CSR_SATP. Reviewed-by: Palmer Dabbelt Signed-off-by: liush IIUC you're supposed to use an actual name. --- arch/riscv/mm/fault.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c index 716d64e..3e560ec13 100644 --- a/arch/riscv/mm/fault.c +++ b/arch/riscv/mm/fault.c @@ -198,6 +198,7 @@ asmlinkage void do_page_fault(struct pt_regs *regs) pmd_t *pmd, *pmd_k; pte_t *pte_k; int index; + unsigned long pfn; /* User mode accesses just cause a SIGSEGV */ if (user_mode(regs)) @@ -212,7 +213,8 @@ asmlinkage void do_page_fault(struct pt_regs *regs) * of a task switch. */ index = pgd_index(addr); - pgd = (pgd_t *)pfn_to_virt(csr_read(CSR_SATP)) + index; + pfn = csr_read(CSR_SATP) & SATP_PPN; + pgd = (pgd_t *)pfn_to_virt(pfn) + index; pgd_k = init_mm.pgd + index; if (!pgd_present(*pgd_k))
Re: KASAN: use-after-free Read in bit_putcs
On 2020/09/27 4:39, Peilin Ye wrote:
> On Sun, Sep 27, 2020 at 01:25:17AM +0900, Tetsuo Handa wrote:
>> Since I don't know the meaning of "struct vt_consize"->v_clin (which is
>> commented
>> with "/* number of pixel rows per character */" but does it mean font size
>> ?),
>> I don't know why we can assign that value to vcp->vc_font.height via
>>
>> if (v.v_clin)
>> vcp->vc_font.height = v.v_clin;
>>
>> in vt_resizex(). While ioctl(PIO_FONT) needs to pass
>> vc->vc_sw->con_font_set()
>> check in con_font_set(), ioctl(VT_RESIZEX) does not pass it in
>> vt_resizex()...
>>
>> Since this problem does not happen if I remove
>>
>> if (v.v_clin)
>> vcp->vc_font.height = v.v_clin;
>
> Hi Tetsuo!
>
>> from vt_resizex(), I guess that some variables are getting confused by
>> change
>> of vc->vc_font.height ...
>
> Yes, see bit_putcs():
>
> (drivers/video/fbdev/core/bitblit.c)
> static void bit_putcs(struct vc_data *vc, struct fb_info *info,
> const unsigned short *s, int count, int yy, int xx,
> int fg, int bg)
> {
> struct fb_image image;
> u32 width = DIV_ROUND_UP(vc->vc_font.width, 8);
> u32 cellsize = width * vc->vc_font.height;
> ^^
>
> `cellsize` is now too large. Later, in bit_putcs_aligned():
>
> while (cnt--) {
> src = vc->vc_font.data + (scr_readw(s++)&
> charmask)*cellsize;
>
>
> `src` goes out of bounds of the data buffer. At first glance I guess
> this is an out-of-bound read reported as a use-after-free read? The
> crashlog says:
How this OOB access is reported varies.
>
> To resolve this out-of-bound issue for now, I think the easiest way
> is to add a range check in bit_putcs(), or bit_putcs_aligned().
>
> ...but yeah, that `VT_RESIZEX` ioctl looks really buggy, and is already
> causing more issues:
At least, since not all fonts have height == 32 (e.g. font_vga_8x8 is height ==
8),
allow changing vc->vc_font.height with only
if (v.v_clin > 32)
return -EINVAL;
validation in VT_RESIZEX looks wrong. This needs more validations.
By the way, can we find a user of VT_RESIZEX? As far as I googled with
"VT_RESIZEX",
I couldn't find a userspace program; only explanation of VT_RESIZEX and kernel
patches
related to VT_RESIZEX are found. Also, while console_ioctl(4) man page says
Any parameter may be set to zero, indicating "no change"
, the assignment
if (!v.v_vlin)
v.v_vlin = vc->vc_scan_lines;
changes the meaning to
If v_vlin parameter is set to zero, the value for associated console is
copied
to each console (instead of preserving current value for that console)
. Maybe for now we can try this (effectively making VT_RESIZEX == VT_RESIZE) ?
vt_ioctl.c | 57 ++---
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index a4e520bdd521..bc33938e2f20 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -773,58 +773,21 @@ static int vt_resizex(struct vc_data *vc, struct
vt_consize __user *cs)
if (copy_from_user(, cs, sizeof(struct vt_consize)))
return -EFAULT;
- /* FIXME: Should check the copies properly */
- if (!v.v_vlin)
- v.v_vlin = vc->vc_scan_lines;
-
- if (v.v_clin) {
- int rows = v.v_vlin / v.v_clin;
- if (v.v_rows != rows) {
- if (v.v_rows) /* Parameters don't add up */
- return -EINVAL;
- v.v_rows = rows;
- }
- }
-
- if (v.v_vcol && v.v_ccol) {
- int cols = v.v_vcol / v.v_ccol;
- if (v.v_cols != cols) {
- if (v.v_cols)
- return -EINVAL;
- v.v_cols = cols;
- }
- }
-
- if (v.v_clin > 32)
- return -EINVAL;
+ if (v.v_vlin)
+ pr_info_once("\"struct vt_consize\"->v_vlin is ignored. Please
report if you need this.\n");
+ if (v.v_clin)
+ pr_info_once("\"struct vt_consize\"->v_clin is ignored. Please
report if you need this.\n");
+ console_lock();
for (i = 0; i < MAX_NR_CONSOLES; i++) {
- struct vc_data *vcp;
+ vc = vc_cons[i].d;
- if (!vc_cons[i].d)
- continue;
- console_lock();
- vcp = vc_cons[i].d;
- if (vcp) {
- int ret;
- int save_scan_lines = vcp->vc_scan_lines;
- int save_font_height = vcp->vc_font.height;
-
- if (v.v_vlin)
-
Re: [PATCH v4 00/17] HSM driver for ACRN hypervisor
Ping...
On 9/22/2020 19:42, shuo.a@intel.com wrote:
> From: Shuo Liu
>
> ACRN is a Type 1 reference hypervisor stack, running directly on the
> bare-metal
> hardware, and is suitable for a variety of IoT and embedded device solutions.
>
> ACRN implements a hybrid VMM architecture, using a privileged Service VM. The
> Service VM manages the system resources (CPU, memory, etc.) and I/O devices of
> User VMs. Multiple User VMs are supported, with each of them running Linux,
> Android OS or Windows. Both Service VM and User VMs are guest VM.
>
> Below figure shows the architecture.
>
> Service VMUser VM
> ++ | +--+
> |+--+| | | |
> ||ACRN userspace|| | | |
> |+--+| | | |
> |-ioctl--| | | | ...
> |kernel space +--+ | | | |
> | | HSM| | | | Drivers |
> | +--+ | | | |
> +|---+ | +--+
> +-hypercall+
> | ACRN Hypervisor|
> +--+
> | Hardware|
> +--+
>
> There is only one Service VM which could run Linux as OS.
>
> In a typical case, the Service VM will be auto started when ACRN Hypervisor is
> booted. Then the ACRN userspace (an application running in Service VM) could
> be
> used to start/stop User VMs by communicating with ACRN Hypervisor Service
> Module (HSM).
>
> ACRN Hypervisor Service Module (HSM) is a middle layer that allows the ACRN
> userspace and Service VM OS kernel to communicate with ACRN Hypervisor
> and manage different User VMs. This middle layer provides the following
> functionalities,
> - Issues hypercalls to the hypervisor to manage User VMs:
> * VM/vCPU management
> * Memory management
> * Device passthrough
> * Interrupts injection
> - I/O requests handling from User VMs.
> - Exports ioctl through HSM char device.
> - Exports function calls for other kernel modules
>
> ACRN is focused on embedded system. So it doesn't support some features.
> E.g.,
> - ACRN doesn't support VM migration.
> - ACRN doesn't support vCPU migration.
>
> This patch set adds the HSM to the Linux kernel.
>
> The basic ARCN support was merged to upstream already.
> https://lore.kernel.org/lkml/1559108037-18813-3-git-send-email-yakui.z...@intel.com/
>
> ChangeLog:
> v4:
> - Used acrn_dev.this_device directly for dev_*() (Reinette)
> - Removed the odd usage of {get|put}_device() on _dev->this_device
> (Greg)
> - Removed unused log code. (Greg)
> - Corrected the return error values. (Greg)
> - Mentioned that HSM relies hypervisor for sanity check in acrn_dev_ioctl()
> comments (Greg)
>
> v3:
> - Used {get|put}_device() helpers on _dev->this_device
> - Moved unused code from front patches to later ones.
> - Removed self-defined pr_fmt() and dev_fmt()
> - Provided comments for acrn_vm_list_lock.
>
> v2:
> - Removed API version related code. (Dave)
> - Replaced pr_*() by dev_*(). (Greg)
> - Used -ENOTTY as the error code of unsupported ioctl. (Greg)
>
> Shuo Liu (16):
> docs: acrn: Introduce ACRN
> x86/acrn: Introduce acrn_{setup, remove}_intr_handler()
> x86/acrn: Introduce hypercall interfaces
> virt: acrn: Introduce ACRN HSM basic driver
> virt: acrn: Introduce VM management interfaces
> virt: acrn: Introduce an ioctl to set vCPU registers state
> virt: acrn: Introduce EPT mapping management
> virt: acrn: Introduce I/O request management
> virt: acrn: Introduce PCI configuration space PIO accesses combiner
> virt: acrn: Introduce interfaces for PCI device passthrough
> virt: acrn: Introduce interrupt injection interfaces
> virt: acrn: Introduce interfaces to query C-states and P-states
> allowed by hypervisor
> virt: acrn: Introduce I/O ranges operation interfaces
> virt: acrn: Introduce ioeventfd
> virt: acrn: Introduce irqfd
> virt: acrn: Introduce an interface for Service VM to control vCPU
>
> Yin Fengwei (1):
> x86/acrn: Introduce an API to check if a VM is privileged
>
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> Documentation/virt/acrn/index.rst | 11 +
> Documentation/virt/acrn/introduction.rst | 40 ++
> Documentation/virt/acrn/io-request.rst| 97 +++
> Documentation/virt/index.rst | 1 +
> MAINTAINERS | 9 +
> arch/x86/include/asm/acrn.h
Re: [PATCH v2] RISC-V: Check clint_time_val before use
On Sat, 26 Sep 2020 03:31:29 PDT (-0700), Damien Le Moal wrote:
On Sat, 2020-09-26 at 15:51 +0530, Anup Patel wrote:
The NoMMU kernel is broken for QEMU virt machine from Linux-5.9-rc6
because the get_cycles() and friends are called very early from
rand_initialize() before CLINT driver is probed. To fix this, we
should check clint_time_val before use in get_cycles() and friends.
I don't think this is the right way to solve that problem, as we're essentially
just lying about the timer rather than informing the system we can't get
timer-based entropy right now. MIPS is explicit about this, I don't see any
reason why we shouldn't be as well.
Does this fix the boot issue (see below for the NULL)? There's some other
random-related arch functions so this might not be quite the right way to do
it.
diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 7f659dda0032..7e39b0068932 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -33,6 +33,18 @@ static inline u32 get_cycles_hi(void)
#define get_cycles_hi get_cycles_hi
#endif /* CONFIG_64BIT */
+/*
+ * Much like MIPS, we may not have a viable counter to use at an early point in
+ * the boot process. Unfortunately we don't have a fallback, so instead we
+ * just return 0.
+ */
+static inline unsigned long random_get_entropy(void)
+{
+ if (unlikely(clint_time_val == NULL))
+ return 0;
+ return get_cycles();
+}
+
#else /* CONFIG_RISCV_M_MODE */
static inline cycles_t get_cycles(void)
Fixes: d5be89a8d118 ("RISC-V: Resurrect the MMIO timer implementation
for M-mode systems")
Signed-off-by: Anup Patel
---
Changes since v1:
- Explicitly initialize clint_time_val to NULL in CLINT driver to
avoid hang on Kendryte K210
---
arch/riscv/include/asm/timex.h| 12 +---
drivers/clocksource/timer-clint.c | 2 +-
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/include/asm/timex.h b/arch/riscv/include/asm/timex.h
index 7f659dda0032..6e7b04874755 100644
--- a/arch/riscv/include/asm/timex.h
+++ b/arch/riscv/include/asm/timex.h
@@ -17,18 +17,24 @@ typedef unsigned long cycles_t;
#ifdef CONFIG_64BIT
static inline cycles_t get_cycles(void)
{
- return readq_relaxed(clint_time_val);
+ if (clint_time_val)
+ return readq_relaxed(clint_time_val);
+ return 0;
}
#else /* !CONFIG_64BIT */
static inline u32 get_cycles(void)
{
- return readl_relaxed(((u32 *)clint_time_val));
+ if (clint_time_val)
+ return readl_relaxed(((u32 *)clint_time_val));
+ return 0;
}
#define get_cycles get_cycles
static inline u32 get_cycles_hi(void)
{
- return readl_relaxed(((u32 *)clint_time_val) + 1);
+ if (clint_time_val)
+ return readl_relaxed(((u32 *)clint_time_val) + 1);
+ return 0;
}
#define get_cycles_hi get_cycles_hi
#endif /* CONFIG_64BIT */
diff --git a/drivers/clocksource/timer-clint.c
b/drivers/clocksource/timer-clint.c
index d17367dee02c..8dbec85979fd 100644
--- a/drivers/clocksource/timer-clint.c
+++ b/drivers/clocksource/timer-clint.c
@@ -37,7 +37,7 @@ static unsigned long clint_timer_freq;
static unsigned int clint_timer_irq;
#ifdef CONFIG_RISCV_M_MODE
-u64 __iomem *clint_time_val;
+u64 __iomem *clint_time_val = NULL;
This one I definately don't get. According the internet, the C standard says
If an object that has static storage duration is not initialized
explicitly, it is initialized implicitly as if every member that has
arithmetic type were assigned 0 and every member that has pointer type were
assigned a null pointer constant.
so unless I'm missing something there shouldn't be any difference between these
two lines. When I just apply this I get exactly the same "objdump -dt" before
and after. I do see some difference in assembly, but only when I don't pass
"-fno-common" and that ends up being passed during my Linux builds.
#endif
static void clint_send_ipi(const struct cpumask *target)
For Kendryte:
Tested-by: Damien Le Moal
--
Damien Le Moal
Western Digital
RE: [PATCH] x86/hyperv: Remove aliases with X64 in their name
From: Joseph Salisbury Sent: Saturday, September 26, 2020 7:26 AM > > In the architecture independent version of hyperv-tlfs.h, commit > c55a844f46f958b > removed the "X64" in the symbol names so they would make sense for both x86 > and > ARM64. That commit added aliases with the "X64" in the x86 version of > hyperv-tlfs.h > so that existing x86 code would continue to compile. > > As a cleanup, update the x86 code to use the symbols without the "X64", then > remove > the aliases. There's no functional change. > > Signed-off-by: Joseph Salisbury > --- > arch/x86/hyperv/hv_init.c | 8 > arch/x86/hyperv/hv_spinlock.c | 2 +- > arch/x86/include/asm/hyperv-tlfs.h | 33 -- > arch/x86/kernel/cpu/mshyperv.c | 8 > arch/x86/kvm/hyperv.c | 20 +- > 5 files changed, 19 insertions(+), 52 deletions(-) > Reviewed-by: Michael Kelley
RE: [PATCH v4 10/11] Driver: hv: util: Use VMBUS_RING_SIZE() for ringbuffer sizes
From: Boqun Feng Sent: Tuesday, September 15, 2020 8:48
PM
>
> For a Hyper-V vmbus, the size of the ringbuffer has two requirements:
>
> 1)it has to take one PAGE_SIZE for the header
>
> 2)it has to be PAGE_SIZE aligned so that double-mapping can work
>
> VMBUS_RING_SIZE() could calculate a correct ringbuffer size which
> fulfills both requirements, therefore use it to make sure vmbus work
> when PAGE_SIZE != HV_HYP_PAGE_SIZE (4K).
>
> Note that since the argument for VMBUS_RING_SIZE() is the size of
> payload (data part), so it will be minus 4k (the size of header when
> PAGE_SIZE = 4k) than the original value to keep the ringbuffer total
> size unchanged when PAGE_SIZE = 4k.
>
> Signed-off-by: Boqun Feng
> Cc: Michael Kelley
>
> ---
> Michael,
>
> I drop your Reviewed-by tag because of the page align issue. Could you
> review this updated version? Thanks!
>
> Regards,
> Boqun
>
>
> drivers/hv/hv_util.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
> index a4e8d96513c2..05566ecdbe4b 100644
> --- a/drivers/hv/hv_util.c
> +++ b/drivers/hv/hv_util.c
> @@ -500,6 +500,9 @@ static void heartbeat_onchannelcallback(void *context)
> }
> }
>
> +#define HV_UTIL_RING_SEND_SIZE VMBUS_RING_SIZE(3 * HV_HYP_PAGE_SIZE)
> +#define HV_UTIL_RING_RECV_SIZE VMBUS_RING_SIZE(3 * HV_HYP_PAGE_SIZE)
> +
> static int util_probe(struct hv_device *dev,
> const struct hv_vmbus_device_id *dev_id)
> {
> @@ -530,8 +533,8 @@ static int util_probe(struct hv_device *dev,
>
> hv_set_drvdata(dev, srv);
>
> - ret = vmbus_open(dev->channel, 4 * HV_HYP_PAGE_SIZE,
> - 4 * HV_HYP_PAGE_SIZE, NULL, 0, srv->util_cb,
> + ret = vmbus_open(dev->channel, HV_UTIL_RING_SEND_SIZE,
> + HV_UTIL_RING_RECV_SIZE, NULL, 0, srv->util_cb,
>dev->channel);
> if (ret)
> goto error;
> @@ -590,8 +593,8 @@ static int util_resume(struct hv_device *dev)
> return ret;
> }
>
> - ret = vmbus_open(dev->channel, 4 * HV_HYP_PAGE_SIZE,
> - 4 * HV_HYP_PAGE_SIZE, NULL, 0, srv->util_cb,
> + ret = vmbus_open(dev->channel, HV_UTIL_RING_SEND_SIZE,
> + HV_UTIL_RING_RECV_SIZE, NULL, 0, srv->util_cb,
>dev->channel);
> return ret;
> }
> --
> 2.28.0
Reviewed-by: Michael Kelley
RE: [PATCH v4 09/11] HID: hyperv: Use VMBUS_RING_SIZE() for ringbuffer sizes
From: Boqun Feng Sent: Tuesday, September 15, 2020 8:48
PM
>
> For a Hyper-V vmbus, the size of the ringbuffer has two requirements:
>
> 1)it has to take one PAGE_SIZE for the header
>
> 2)it has to be PAGE_SIZE aligned so that double-mapping can work
>
> VMBUS_RING_SIZE() could calculate a correct ringbuffer size which
> fulfills both requirements, therefore use it to make sure vmbus work
> when PAGE_SIZE != HV_HYP_PAGE_SIZE (4K).
>
> Note that since the argument for VMBUS_RING_SIZE() is the size of
> payload (data part), so it will be minus 4k (the size of header when
> PAGE_SIZE = 4k) than the original value to keep the ringbuffer total
> size unchanged when PAGE_SIZE = 4k.
>
> Signed-off-by: Boqun Feng
> Cc: Jiri Kosina
> Cc: Michael Kelley
> ---
> Michael and Jiri,
>
> I change the code because of a problem I found:
>
>
> https://lore.kernel.org/lkml/20200914084600.ga45...@debian-boqun.qqnc3lrjykvubdpftowmye0fmh.lx.internal.cloudapp.net/
>
> , so I drop your Reviewed-by or Acked-by tag. If the update version
> looks good to you, may I add your tag again? Thanks in advance, and
> apologies for the inconvenience.
>
> Regards,
> Boqun
>
> drivers/hid/hid-hyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> index 0b6ee1dee625..978ee2aab2d4 100644
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -104,8 +104,8 @@ struct synthhid_input_report {
>
> #pragma pack(pop)
>
> -#define INPUTVSC_SEND_RING_BUFFER_SIZE (40 * 1024)
> -#define INPUTVSC_RECV_RING_BUFFER_SIZE (40 * 1024)
> +#define INPUTVSC_SEND_RING_BUFFER_SIZE VMBUS_RING_SIZE(36 * 1024)
> +#define INPUTVSC_RECV_RING_BUFFER_SIZE VMBUS_RING_SIZE(36 * 1024)
>
>
> enum pipe_prot_msg_type {
> --
> 2.28.0
Reviewed-by: Michael Kelley
RE: [PATCH v4 08/11] Input: hyperv-keyboard: Use VMBUS_RING_SIZE() for ringbuffer sizes
From: Boqun Feng Sent: Tuesday, September 15, 2020 8:48
PM
>
> For a Hyper-V vmbus, the size of the ringbuffer has two requirements:
>
> 1)it has to take one PAGE_SIZE for the header
>
> 2)it has to be PAGE_SIZE aligned so that double-mapping can work
>
> VMBUS_RING_SIZE() could calculate a correct ringbuffer size which
> fulfills both requirements, therefore use it to make sure vmbus work
> when PAGE_SIZE != HV_HYP_PAGE_SIZE (4K).
>
> Note that since the argument for VMBUS_RING_SIZE() is the size of
> payload (data part), so it will be minus 4k (the size of header when
> PAGE_SIZE = 4k) than the original value to keep the ringbuffer total
> size unchanged when PAGE_SIZE = 4k.
>
> Signed-off-by: Boqun Feng
> Cc: Michael Kelley
> Cc: Dmitry Torokhov
> ---
> Michael and Dmitry,
>
> I change the code because of a problem I found:
>
>
> https://lore.kernel.org/lkml/20200914084600.ga45...@debian-boqun.qqnc3lrjykvubdpftowmye0fmh.lx.internal.cloudapp.net/
>
> , so I drop your Reviewed-by or Acked-by tag. If the update version
> looks good to you, may I add your tag again? Thanks in advance, and
> apologies for the inconvenience.
>
> Regards,
> Boqun
>
> drivers/input/serio/hyperv-keyboard.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/serio/hyperv-keyboard.c
> b/drivers/input/serio/hyperv-keyboard.c
> index df4e9f6f4529..1a7b72a9016d 100644
> --- a/drivers/input/serio/hyperv-keyboard.c
> +++ b/drivers/input/serio/hyperv-keyboard.c
> @@ -75,8 +75,8 @@ struct synth_kbd_keystroke {
>
> #define HK_MAXIMUM_MESSAGE_SIZE 256
>
> -#define KBD_VSC_SEND_RING_BUFFER_SIZE(40 * 1024)
> -#define KBD_VSC_RECV_RING_BUFFER_SIZE(40 * 1024)
> +#define KBD_VSC_SEND_RING_BUFFER_SIZEVMBUS_RING_SIZE(36 * 1024)
> +#define KBD_VSC_RECV_RING_BUFFER_SIZEVMBUS_RING_SIZE(36 * 1024)
>
> #define XTKBD_EMUL0 0xe0
> #define XTKBD_EMUL1 0xe1
> --
> 2.28.0
Reviewed-by: Michael Kelley
[PATCH] kernel/futex.c: fix incorrect 'should_fail_futex' handling
From: Mateusz Nosek
Previously if 'futex_should_fail' returned true, then only 'ret' variable
was set, which was later overwritten without being read. The patch fixes
the problem.
Signed-off-by: Mateusz Nosek
---
kernel/futex.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/futex.c b/kernel/futex.c
index a5876694a60e..39681bf8b06c 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -1502,8 +1502,10 @@ static int wake_futex_pi(u32 __user *uaddr, u32 uval,
struct futex_pi_state *pi_
*/
newval = FUTEX_WAITERS | task_pid_vnr(new_owner);
- if (unlikely(should_fail_futex(true)))
+ if (unlikely(should_fail_futex(true))) {
ret = -EFAULT;
+ goto out_unlock;
+ }
ret = cmpxchg_futex_value_locked(, uaddr, uval, newval);
if (!ret && (curval != uval)) {
--
2.20.1
Re: [PATCH v2 3/4] mm: Do early cow for pinned pages during fork() for ptes
On Sat, Sep 26, 2020 at 4:23 PM Jason Gunthorpe wrote: > > Linus's version doesn't do pte_sw_mkyoung(), but looks OK to have it I don't think it matters. But I don't think it should make it young, since there's no access, but it's not like it's a big deal. > > + pte = maybe_mkwrite(pte_mkdirty(pte), new); > > maybe_mkwrite() was not in Linus's version but it is wp_page_copy(). Actually, it is in my version too, just in a different form. I did it using if (vma->vm_flags & VM_WRITE) *src_pte = pte_mkwrite(*src_pte); instead, ie avoiding the write to src_pte if it wasn't a writable vma (and I had checked that it was dirty and not done this at all if not, so no need for the mkdirty). > It seemed like mk_pte() should set the proper write > bit already from the vm_page_prot? No, vm_page_prot won't have the writable bit for a COW mapping. The write bit gets set when the write happens (which may be on the first access, of course), by the code that makes sure it's a private copy. > Perhaps this is harmless but redundant? No, the pte_mkwrite() is required in some form, whether it's that "maybe_mkwrite()" that looks at the vm flags, or in that explicit form. > > + page_add_new_anon_rmap(new_page, new, addr, > > false); > > + rss[mm_counter(new_page)]++; > > + set_pte_at(dst_mm, addr, dst_pte, pte); > > Linus's patch had a lru_cache_add_inactive_or_unevictable() here, like > wp_page_copy() Yeah, I do think that is needed so that we have the new page on the LRU and it gets properly evicted under memory pressure. Linus
Re: [PATCH v10 10/16] s390/vfio-ap: allow configuration of matrix mdev in use by a KVM guest
On Fri, 21 Aug 2020 15:56:10 -0400 Tony Krowiak wrote: > The current support for pass-through crypto adapters does not allow > configuration of a matrix mdev when it is in use by a KVM guest. Let's > allow AP resources - i.e., adapters, domains and control domains - to be > assigned to or unassigned from a matrix mdev while it is in use by a guest. > This is in preparation for the introduction of support for dynamic > configuration of the AP matrix for a running KVM guest. AFAIU this will let the user do the assign, which will however only take effect if the same mdev is re-used with a freshly constructed VM, or? This is however supposed to change real soon (in patch 11). From the perspective of bisectability we would end up with a single commit that acts funny. How about switching up patches 10 and 11. This way the changes you have in the current 11 would remain dormant until the changes in the current 10 enable the complete new feature (hotplug)? > > Signed-off-by: Tony Krowiak > --- > drivers/s390/crypto/vfio_ap_ops.c | 24 > 1 file changed, 24 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_ops.c > b/drivers/s390/crypto/vfio_ap_ops.c > index 24fd47e43b80..cf3321eb239b 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -773,10 +773,6 @@ static ssize_t assign_adapter_store(struct device *dev, > struct mdev_device *mdev = mdev_from_dev(dev); > struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev); > > - /* If the guest is running, disallow assignment of adapter */ > - if (matrix_mdev->kvm) > - return -EBUSY; > - > ret = kstrtoul(buf, 0, ); > if (ret) > return ret; > @@ -828,10 +824,6 @@ static ssize_t unassign_adapter_store(struct device *dev, > struct mdev_device *mdev = mdev_from_dev(dev); > struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev); > > - /* If the guest is running, disallow un-assignment of adapter */ > - if (matrix_mdev->kvm) > - return -EBUSY; > - > ret = kstrtoul(buf, 0, ); > if (ret) > return ret; > @@ -891,10 +883,6 @@ static ssize_t assign_domain_store(struct device *dev, > struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev); > unsigned long max_apqi = matrix_mdev->matrix.aqm_max; > > - /* If the guest is running, disallow assignment of domain */ > - if (matrix_mdev->kvm) > - return -EBUSY; > - > ret = kstrtoul(buf, 0, ); > if (ret) > return ret; > @@ -946,10 +934,6 @@ static ssize_t unassign_domain_store(struct device *dev, > struct mdev_device *mdev = mdev_from_dev(dev); > struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev); > > - /* If the guest is running, disallow un-assignment of domain */ > - if (matrix_mdev->kvm) > - return -EBUSY; > - > ret = kstrtoul(buf, 0, ); > if (ret) > return ret; > @@ -991,10 +975,6 @@ static ssize_t assign_control_domain_store(struct device > *dev, > struct mdev_device *mdev = mdev_from_dev(dev); > struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev); > > - /* If the guest is running, disallow assignment of control domain */ > - if (matrix_mdev->kvm) > - return -EBUSY; > - > ret = kstrtoul(buf, 0, ); > if (ret) > return ret; > @@ -1036,10 +1016,6 @@ static ssize_t unassign_control_domain_store(struct > device *dev, > struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev); > unsigned long max_domid = matrix_mdev->matrix.adm_max; > > - /* If the guest is running, disallow un-assignment of control domain */ > - if (matrix_mdev->kvm) > - return -EBUSY; > - > ret = kstrtoul(buf, 0, ); > if (ret) > return ret;
[PATCH 5/5] mfd: tps65910: remove unused pointers
Client pointers in tps65910 data are not used in the drivers.
Remove those fields.
Signed-off-by: Michał Mirosław
---
include/linux/mfd/tps65910.h | 5 -
1 file changed, 5 deletions(-)
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index f7398d982f23..701925db75b3 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -890,11 +890,6 @@ struct tps65910 {
struct regmap *regmap;
unsigned long id;
- /* Client devices */
- struct tps65910_pmic *pmic;
- struct tps65910_rtc *rtc;
- struct tps65910_power *power;
-
/* Device node parsed board data */
struct tps65910_board *of_plat_data;
--
2.20.1
[PATCH 1/5] gpio: tps65910: use regmap accessors
Use regmap accessors directly for register manipulation - removing one layer of abstraction. Signed-off-by: Michał Mirosław --- drivers/gpio/gpio-tps65910.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpio/gpio-tps65910.c b/drivers/gpio/gpio-tps65910.c index 0c785b0fd161..0c0b445c75c0 100644 --- a/drivers/gpio/gpio-tps65910.c +++ b/drivers/gpio/gpio-tps65910.c @@ -28,7 +28,7 @@ static int tps65910_gpio_get(struct gpio_chip *gc, unsigned offset) struct tps65910 *tps65910 = tps65910_gpio->tps65910; unsigned int val; - tps65910_reg_read(tps65910, TPS65910_GPIO0 + offset, ); + regmap_read(tps65910->regmap, TPS65910_GPIO0 + offset, ); if (val & GPIO_STS_MASK) return 1; @@ -43,10 +43,10 @@ static void tps65910_gpio_set(struct gpio_chip *gc, unsigned offset, struct tps65910 *tps65910 = tps65910_gpio->tps65910; if (value) - tps65910_reg_set_bits(tps65910, TPS65910_GPIO0 + offset, + regmap_set_bits(tps65910->regmap, TPS65910_GPIO0 + offset, GPIO_SET_MASK); else - tps65910_reg_clear_bits(tps65910, TPS65910_GPIO0 + offset, + regmap_clear_bits(tps65910->regmap, TPS65910_GPIO0 + offset, GPIO_SET_MASK); } @@ -59,7 +59,7 @@ static int tps65910_gpio_output(struct gpio_chip *gc, unsigned offset, /* Set the initial value */ tps65910_gpio_set(gc, offset, value); - return tps65910_reg_set_bits(tps65910, TPS65910_GPIO0 + offset, + return regmap_set_bits(tps65910->regmap, TPS65910_GPIO0 + offset, GPIO_CFG_MASK); } @@ -68,7 +68,7 @@ static int tps65910_gpio_input(struct gpio_chip *gc, unsigned offset) struct tps65910_gpio *tps65910_gpio = gpiochip_get_data(gc); struct tps65910 *tps65910 = tps65910_gpio->tps65910; - return tps65910_reg_clear_bits(tps65910, TPS65910_GPIO0 + offset, + return regmap_clear_bits(tps65910->regmap, TPS65910_GPIO0 + offset, GPIO_CFG_MASK); } @@ -157,7 +157,7 @@ static int tps65910_gpio_probe(struct platform_device *pdev) if (!pdata->en_gpio_sleep[i]) continue; - ret = tps65910_reg_set_bits(tps65910, + ret = regmap_set_bits(tps65910->regmap, TPS65910_GPIO0 + i, GPIO_SLEEP_MASK); if (ret < 0) dev_warn(tps65910->dev, -- 2.20.1
[PATCH 2/5] regulator: tps65910: use regmap accessors
Use regmap accessors directly for register manipulation - removing
one layer of abstraction.
Signed-off-by: Michał Mirosław
---
drivers/regulator/tps65910-regulator.c | 125 +
1 file changed, 63 insertions(+), 62 deletions(-)
diff --git a/drivers/regulator/tps65910-regulator.c
b/drivers/regulator/tps65910-regulator.c
index faa5b3538167..1d5b0a1b86f7 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -390,8 +390,8 @@ static int tps65911_get_ctrl_register(int id)
static int tps65910_set_mode(struct regulator_dev *dev, unsigned int mode)
{
struct tps65910_reg *pmic = rdev_get_drvdata(dev);
- struct tps65910 *mfd = pmic->mfd;
- int reg, value, id = rdev_get_id(dev);
+ struct regmap *regmap = rdev_get_regmap(dev);
+ int reg, id = rdev_get_id(dev);
reg = pmic->get_ctrl_reg(id);
if (reg < 0)
@@ -399,14 +399,14 @@ static int tps65910_set_mode(struct regulator_dev *dev,
unsigned int mode)
switch (mode) {
case REGULATOR_MODE_NORMAL:
- return tps65910_reg_update_bits(pmic->mfd, reg,
- LDO_ST_MODE_BIT | LDO_ST_ON_BIT,
- LDO_ST_ON_BIT);
+ return regmap_update_bits(regmap, reg,
+ LDO_ST_MODE_BIT | LDO_ST_ON_BIT,
+ LDO_ST_ON_BIT);
case REGULATOR_MODE_IDLE:
- value = LDO_ST_ON_BIT | LDO_ST_MODE_BIT;
- return tps65910_reg_set_bits(mfd, reg, value);
+ return regmap_set_bits(regmap, reg,
+ LDO_ST_ON_BIT | LDO_ST_MODE_BIT);
case REGULATOR_MODE_STANDBY:
- return tps65910_reg_clear_bits(mfd, reg, LDO_ST_ON_BIT);
+ return regmap_clear_bits(regmap, reg, LDO_ST_ON_BIT);
}
return -EINVAL;
@@ -415,13 +415,14 @@ static int tps65910_set_mode(struct regulator_dev *dev,
unsigned int mode)
static unsigned int tps65910_get_mode(struct regulator_dev *dev)
{
struct tps65910_reg *pmic = rdev_get_drvdata(dev);
+ struct regmap *regmap = rdev_get_regmap(dev);
int ret, reg, value, id = rdev_get_id(dev);
reg = pmic->get_ctrl_reg(id);
if (reg < 0)
return reg;
- ret = tps65910_reg_read(pmic->mfd, reg, );
+ ret = regmap_read(regmap, reg, );
if (ret < 0)
return ret;
@@ -435,20 +436,20 @@ static unsigned int tps65910_get_mode(struct
regulator_dev *dev)
static int tps65910_get_voltage_dcdc_sel(struct regulator_dev *dev)
{
- struct tps65910_reg *pmic = rdev_get_drvdata(dev);
+ struct regmap *regmap = rdev_get_regmap(dev);
int ret, id = rdev_get_id(dev);
int opvsel = 0, srvsel = 0, vselmax = 0, mult = 0, sr = 0;
switch (id) {
case TPS65910_REG_VDD1:
- ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD1_OP, );
+ ret = regmap_read(regmap, TPS65910_VDD1_OP, );
if (ret < 0)
return ret;
- ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD1, );
+ ret = regmap_read(regmap, TPS65910_VDD1, );
if (ret < 0)
return ret;
mult = (mult & VDD1_VGAIN_SEL_MASK) >> VDD1_VGAIN_SEL_SHIFT;
- ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD1_SR, );
+ ret = regmap_read(regmap, TPS65910_VDD1_SR, );
if (ret < 0)
return ret;
sr = opvsel & VDD1_OP_CMD_MASK;
@@ -457,14 +458,14 @@ static int tps65910_get_voltage_dcdc_sel(struct
regulator_dev *dev)
vselmax = 75;
break;
case TPS65910_REG_VDD2:
- ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD2_OP, );
+ ret = regmap_read(regmap, TPS65910_VDD2_OP, );
if (ret < 0)
return ret;
- ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD2, );
+ ret = regmap_read(regmap, TPS65910_VDD2, );
if (ret < 0)
return ret;
mult = (mult & VDD2_VGAIN_SEL_MASK) >> VDD2_VGAIN_SEL_SHIFT;
- ret = tps65910_reg_read(pmic->mfd, TPS65910_VDD2_SR, );
+ ret = regmap_read(regmap, TPS65910_VDD2_SR, );
if (ret < 0)
return ret;
sr = opvsel & VDD2_OP_CMD_MASK;
@@ -473,12 +474,10 @@ static int tps65910_get_voltage_dcdc_sel(struct
regulator_dev *dev)
vselmax = 75;
break;
case TPS65911_REG_VDDCTRL:
- ret = tps65910_reg_read(pmic->mfd, TPS65911_VDDCTRL_OP,
- );
+ ret = regmap_read(regmap, TPS65911_VDDCTRL_OP, );
if (ret < 0)
[PATCH 3/5] mfd: tps65911-comparator: use regmap accessors
Use regmap accessors directly for register manipulation - removing one layer of abstraction. Signed-off-by: Michał Mirosław --- drivers/mfd/tps65911-comparator.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/mfd/tps65911-comparator.c b/drivers/mfd/tps65911-comparator.c index 2334cd61c98d..8f4210075913 100644 --- a/drivers/mfd/tps65911-comparator.c +++ b/drivers/mfd/tps65911-comparator.c @@ -69,7 +69,7 @@ static int comp_threshold_set(struct tps65910 *tps65910, int id, int voltage) return -EINVAL; val = index << 1; - ret = tps65910_reg_write(tps65910, tps_comp.reg, val); + ret = regmap_write(tps65910->regmap, tps_comp.reg, val); return ret; } @@ -80,7 +80,7 @@ static int comp_threshold_get(struct tps65910 *tps65910, int id) unsigned int val; int ret; - ret = tps65910_reg_read(tps65910, tps_comp.reg, ); + ret = regmap_read(tps65910->regmap, tps_comp.reg, ); if (ret < 0) return ret; -- 2.20.1
[PATCH 4/5] mfd: tps65910: clean up after switching to regmap
Remove wrappers around regmap calls to remove now-useless indirection.
Signed-off-by: Michał Mirosław
---
drivers/mfd/tps65910.c | 16
include/linux/mfd/tps65910.h | 35 ---
2 files changed, 8 insertions(+), 43 deletions(-)
diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 11959021b50a..36a52f44cd11 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -292,7 +292,7 @@ static int tps65910_ck32k_init(struct tps65910 *tps65910,
if (!pmic_pdata->en_ck32k_xtal)
return 0;
- ret = tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+ ret = regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_CK32K_CTRL_MASK);
if (ret < 0) {
dev_err(tps65910->dev, "clear ck32k_ctrl failed: %d\n", ret);
@@ -314,7 +314,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
dev = tps65910->dev;
/* enabling SLEEP device state */
- ret = tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL,
+ ret = regmap_set_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_DEV_SLP_MASK);
if (ret < 0) {
dev_err(dev, "set dev_slp failed: %d\n", ret);
@@ -322,7 +322,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
}
if (pmic_pdata->slp_keepon.therm_keepon) {
- ret = tps65910_reg_set_bits(tps65910,
+ ret = regmap_set_bits(tps65910->regmap,
TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_THERM_KEEPON_MASK);
if (ret < 0) {
@@ -332,7 +332,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
}
if (pmic_pdata->slp_keepon.clkout32k_keepon) {
- ret = tps65910_reg_set_bits(tps65910,
+ ret = regmap_set_bits(tps65910->regmap,
TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_CLKOUT32K_KEEPON_MASK);
if (ret < 0) {
@@ -342,7 +342,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
}
if (pmic_pdata->slp_keepon.i2chs_keepon) {
- ret = tps65910_reg_set_bits(tps65910,
+ ret = regmap_set_bits(tps65910->regmap,
TPS65910_SLEEP_KEEP_RES_ON,
SLEEP_KEEP_RES_ON_I2CHS_KEEPON_MASK);
if (ret < 0) {
@@ -354,7 +354,7 @@ static int tps65910_sleepinit(struct tps65910 *tps65910,
return 0;
disable_dev_slp:
- tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+ regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_DEV_SLP_MASK);
err_sleep_init:
@@ -436,11 +436,11 @@ static void tps65910_power_off(void)
tps65910 = dev_get_drvdata(_i2c_client->dev);
- if (tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL,
+ if (regmap_set_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_PWR_OFF_MASK) < 0)
return;
- tps65910_reg_clear_bits(tps65910, TPS65910_DEVCTRL,
+ regmap_clear_bits(tps65910->regmap, TPS65910_DEVCTRL,
DEVCTRL_DEV_ON_MASK);
}
diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h
index ce4b9e743f7c..f7398d982f23 100644
--- a/include/linux/mfd/tps65910.h
+++ b/include/linux/mfd/tps65910.h
@@ -913,39 +913,4 @@ static inline int tps65910_chip_id(struct tps65910
*tps65910)
return tps65910->id;
}
-static inline int tps65910_reg_read(struct tps65910 *tps65910, u8 reg,
- unsigned int *val)
-{
- return regmap_read(tps65910->regmap, reg, val);
-}
-
-static inline int tps65910_reg_write(struct tps65910 *tps65910, u8 reg,
- unsigned int val)
-{
- return regmap_write(tps65910->regmap, reg, val);
-}
-
-static inline int tps65910_reg_set_bits(struct tps65910 *tps65910, u8 reg,
- u8 mask)
-{
- return regmap_update_bits(tps65910->regmap, reg, mask, mask);
-}
-
-static inline int tps65910_reg_clear_bits(struct tps65910 *tps65910, u8 reg,
- u8 mask)
-{
- return regmap_update_bits(tps65910->regmap, reg, mask, 0);
-}
-
-static inline int tps65910_reg_update_bits(struct tps65910 *tps65910, u8 reg,
- u8 mask, u8 val)
-{
- return regmap_update_bits(tps65910->regmap, reg, mask, val);
-}
-
-static inline int tps65910_irq_get_virq(struct tps65910 *tps65910, int irq)
-{
- return regmap_irq_get_virq(tps65910->irq_data, irq);
-}
-
#endif /* __LINUX_MFD_TPS65910_H */
--
2.20.1
[PATCH 0/5] tps65910: cleanup regmap use
tps65910 was converted a long time ago to regmap. This series cleans up after the conversion by removing tps65910_reg_*() indirections and other unused fields in MFD structure. Michał Mirosław (5): gpio: tps65910: use regmap accessors regulator: tps65910: use regmap accessors mfd: tps65911-comparator: use regmap accessors mfd: tps65910: clean up after switching to regmap mfd: tps65910: remove unused pointers drivers/gpio/gpio-tps65910.c | 12 +-- drivers/mfd/tps65910.c | 16 ++-- drivers/mfd/tps65911-comparator.c | 4 +- drivers/regulator/tps65910-regulator.c | 125 + include/linux/mfd/tps65910.h | 40 5 files changed, 79 insertions(+), 118 deletions(-) -- 2.20.1
Re: Can we credibly make vdso_sgx_enter_enclave() pleasant to use?
On 9/26/2020 12:05 PM, Andy Lutomirski wrote:
On Fri, Sep 25, 2020 at 3:29 PM Sean Christopherson
wrote:
On Fri, Sep 25, 2020 at 01:20:03PM -0700, Andy Lutomirski wrote:
On Fri, Sep 25, 2020 at 12:09 PM Sean Christopherson
wrote:
But where would the vDSO get memory for that little data structure? It can't
be percpu because the current task can get preempted. It can't be per instance
of the vDSO because a single mm/process can have multiple tasks entering an
enclave. Per task might work, but how would the vDSO get that info? E.g.
via a syscall, which seems like complete overkill?
The stack.
Duh.
The vDSO could, logically, do:
struct sgx_entry_state {
unsigned long real_rbp;
unsigned long real_rsp;
unsigned long orig_fsbase;
};
...
struct sgx_entry_state state;
state.rbp = rbp; [ hey, this is pseudocode. the real code would be in asm.]
state.rsp = rsp;
state.fsbase = __rdfsbase();
rbp = arg->rbp;
/* set up all other regs */
wrfsbase %rsp
movq enclave_rsp(%rsp), %rsp
I think this is where there's a disconnect with what is being requested by the
folks writing run times. IIUC, they want to use the untrusted runtime's stack
to pass params because it doesn't require additional memory allocations and
automagically grows as necessary (obviously to a certain limit). I.e. forcing
the caller to provide an alternative "stack" defeats the purpose of using the
untrusted stack.
I personally find this concept rather distasteful. Sure, it might
save a couple cycles, but it means that the enclave has hardcoded some
kind of assumption about the outside-the-enclave stack.
It's more than just a couple of cycles. It's convenience. Yes, an
enclave may overflow the caller's stack with big allocations but those
are rare. In more common cases less than the red zone size (128 bytes)
are required. And we should optimize for the more common cases.
And yes again, the enclave has to assume something about the stack. But
please note that the vDSO has to save its "context" somewhere so that it
can switch back to it. The "context" currently is anchored at RBP so the
enclave has to preserve it. If not RBP, the "context" has to anchor
"something else", and we have to assume the enclave preserve that
"something else". That said, we can't get rid of assumptions. RBP is a
reasonable choice because it is simple without obvious side effects,
i.e. most compilers/ABIs preserve RBP so developers don't have to pay
extra attention to it generally.
If I were asked to opine on the API, I'd say I like the most the initial
version with callback support. The stack parameters were easier to
set/retrieve than struct members (requiring hand-crafted offset macros)
in asm, and didn't need any padding. The callback was easy to use
(non-NULL pointer) or skip (NULL pointer). Standard/unified error codes
were easier to handle than separate error/exit_reason. Additional data
for callback could be captured in a structure enclosing
sgx_enclave_exception so no need to be explicitly passed (languages that
don't support offsetof/container_of can always employ an asm wrapper).
The current API looks confusing and overly complicated to me, even
though it still works.
Given that RBP seems reasonably likely to be stable across enclave
executions, I suppose we could add a flag and an RSP value in the
sgx_enclave_run structure. If set, the vDSO would swap out RSP (but
not RBP) with the provided value on entry and record the new RSP on
exit. I don't know if this would be useful to people.
I would say, if one wants to use a different untrusted stack for calling
the enclave, he/she could switch stack before calling vDSO. Given this
isn't commonly required, I vote NO here.
I do think we need to add at least minimal CFI annotations no matter what we do.
Can't agree more.
Re: [PATCH v10 09/16] s390/vfio-ap: allow assignment of unavailable AP queues to mdev device
On Fri, 21 Aug 2020 15:56:09 -0400
Tony Krowiak wrote:
> The current implementation does not allow assignment of an AP adapter or
> domain to an mdev device if the APQNs resulting from the assignment
> do not reference AP queue devices that are bound to the vfio_ap device
> driver. This patch allows assignment of AP resources to the matrix mdev as
> long as the APQNs resulting from the assignment:
>1. Are not reserved by the AP BUS for use by the zcrypt device drivers.
>2. Are not assigned to another matrix mdev.
>
> The rationale behind this is twofold:
>1. The AP architecture does not preclude assignment of APQNs to an AP
> configuration that are not available to the system.
>2. APQNs that do not reference a queue device bound to the vfio_ap
> device driver will not be assigned to the guest's CRYCB, so the
> guest will not get access to queues not bound to the vfio_ap driver.
>
> Signed-off-by: Tony Krowiak
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 212 +-
> 1 file changed, 35 insertions(+), 177 deletions(-)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c
> b/drivers/s390/crypto/vfio_ap_ops.c
> index eaf4e9eab6cb..24fd47e43b80 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -1,4 +1,3 @@
> -// SPDX-License-Identifier: GPL-2.0+
Probably not intentional, or?
> /*
> * Adjunct processor matrix VFIO device driver callbacks.
> *
> @@ -420,122 +419,6 @@ static struct attribute_group
> *vfio_ap_mdev_type_groups[] = {
> NULL,
> };
>
> -struct vfio_ap_queue_reserved {
> - unsigned long *apid;
> - unsigned long *apqi;
> - bool reserved;
> -};
> -
> -/**
> - * vfio_ap_has_queue
> - *
> - * @dev: an AP queue device
> - * @data: a struct vfio_ap_queue_reserved reference
> - *
> - * Flags whether the AP queue device (@dev) has a queue ID containing the
> APQN,
> - * apid or apqi specified in @data:
> - *
> - * - If @data contains both an apid and apqi value, then @data will be
> flagged
> - * as reserved if the APID and APQI fields for the AP queue device matches
> - *
> - * - If @data contains only an apid value, @data will be flagged as
> - * reserved if the APID field in the AP queue device matches
> - *
> - * - If @data contains only an apqi value, @data will be flagged as
> - * reserved if the APQI field in the AP queue device matches
> - *
> - * Returns 0 to indicate the input to function succeeded. Returns -EINVAL if
> - * @data does not contain either an apid or apqi.
> - */
> -static int vfio_ap_has_queue(struct device *dev, void *data)
> -{
> - struct vfio_ap_queue_reserved *qres = data;
> - struct ap_queue *ap_queue = to_ap_queue(dev);
> - ap_qid_t qid;
> - unsigned long id;
> -
> - if (qres->apid && qres->apqi) {
> - qid = AP_MKQID(*qres->apid, *qres->apqi);
> - if (qid == ap_queue->qid)
> - qres->reserved = true;
> - } else if (qres->apid && !qres->apqi) {
> - id = AP_QID_CARD(ap_queue->qid);
> - if (id == *qres->apid)
> - qres->reserved = true;
> - } else if (!qres->apid && qres->apqi) {
> - id = AP_QID_QUEUE(ap_queue->qid);
> - if (id == *qres->apqi)
> - qres->reserved = true;
> - } else {
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> -/**
> - * vfio_ap_verify_queue_reserved
> - *
> - * @matrix_dev: a mediated matrix device
> - * @apid: an AP adapter ID
> - * @apqi: an AP queue index
> - *
> - * Verifies that the AP queue with @apid/@apqi is reserved by the VFIO AP
> device
> - * driver according to the following rules:
> - *
> - * - If both @apid and @apqi are not NULL, then there must be an AP queue
> - * device bound to the vfio_ap driver with the APQN identified by @apid and
> - * @apqi
> - *
> - * - If only @apid is not NULL, then there must be an AP queue device bound
> - * to the vfio_ap driver with an APQN containing @apid
> - *
> - * - If only @apqi is not NULL, then there must be an AP queue device bound
> - * to the vfio_ap driver with an APQN containing @apqi
> - *
> - * Returns 0 if the AP queue is reserved; otherwise, returns -EADDRNOTAVAIL.
> - */
> -static int vfio_ap_verify_queue_reserved(unsigned long *apid,
> - unsigned long *apqi)
> -{
> - int ret;
> - struct vfio_ap_queue_reserved qres;
> -
> - qres.apid = apid;
> - qres.apqi = apqi;
> - qres.reserved = false;
> -
> - ret = driver_for_each_device(_dev->vfio_ap_drv->driver, NULL,
> - , vfio_ap_has_queue);
> - if (ret)
> - return ret;
> -
> - if (qres.reserved)
> - return 0;
> -
> - return -EADDRNOTAVAIL;
> -}
> -
> -static int
> -vfio_ap_mdev_verify_queues_reserved_for_apid(struct ap_matrix_mdev
> *matrix_mdev,
> -
FORM.MRS.MARYAM C. RICHARD.
My Beloved Friend In The Lord. Greetings in the name of our Lord Jesus Christ. I am Mrs. Maryam C. Richard, From Poland, a widow to late (MR.RICHARD BURSON from Florida , U.S.A) l am 51 years old and I am a converted born again Christian, suffering from long term Cancer of the KIDNEY, from all indication my condition is really deteriorating and it is quite obvious that I might not live more than two (2) months, according to my Doctor because the cancer has gotten to a very worst / dangerous stage. My late husband and my only child died last five years ago, his death was politically motivated. My late husband was a very rich and wealthy business man who was running his Gold/Diamond Business here in Burkina Faso. After his death, I inherited all his business and wealth. My doctors have advised me that I may not live for more than two (2) months, so I now decided to divide the part of this wealth, to contribute to the development of the churches in Africa, America, Asia, and Europe. I got your email id from your country guestbook, and I prayed over it and the spirit our Lord Jesus directed me to you as an honest person who can assist me to fulfill my wish here on earth before I give up in live. My late husband, have an account deposited the sum of $5.3 Million Dollars in BANK OF AFRICA Burkina Faso where he do his business projects before his death, So I want the Sum $5.3 Million Dollars in BANK OF AFRICA Burkina Faso to be release/transfer to you as the less privileged because I cannot take this money to the grave. Please I want you to note that this fund is lodged in a Bank Of Africa in Burkina Faso. Once I hear from you, I will forward to you all the information's you will use to get this fund released from the bank of Africa and to be transferred to your bank account. I honestly pray that this money when transferred to you will be used for the said purpose on Churches and Orphanage because l have come to find out that wealth acquisition without Christ is vanity. May the grace of our lord Jesus the love of God and the fellowship of God be with you and your family as you will use part of this sum for Churches and Orphanage for my soul to rest in peace when I die. Urgently Reply with the information’s bellow to this My Private E-mail bellow: ( mrscompaoremary...@gmail.com ) 1. YOUR FULL NAME.. 2. NATIONALITY. 3. YOUR AGE.. 4. OCCUPATION. 5. PHONE NUMBER. BEST REGARD. MRS.MARYAM C. RICHARD.
Re: [GIT PULL] devfreq fixes for v5.9-rc7
On Fri, Sep 25, 2020 at 11:38 PM Rafael J. Wysocki wrote: > > On Thu, Sep 24, 2020 at 12:03 PM Chanwoo Choi wrote: > > > > Dear Rafael, > > > > This is devfreq-next pull request for v5.9-rc7. I add detailed description > > of > > this pull request on the following tag. Please pull devfreq with following > > updates. > > - tag name : devfreq-fixes-for-5.9-rc7 > > Pulled, thanks! > > I would appreciate receiving pull requests a bit earlier before the > target -rc, though. I'm sorry for late pull request for rc. I'll send pull request as you commented. -- Best Regards, Chanwoo Choi
RE: [PATCH] Revert "net: ethernet: ixgbe: check the return value of ixgbe_mii_bus_init()"
> -Original Message- > From: Bartosz Golaszewski > Sent: Friday, September 25, 2020 17:44 > To: Liu, Yongxin > Cc: David S . Miller ; netdev > ; LKML > Subject: Re: [PATCH] Revert "net: ethernet: ixgbe: check the return value > of ixgbe_mii_bus_init()" > > On Fri, Sep 25, 2020 at 10:51 AM Liu, Yongxin > wrote: > > > > [snip] > > > > > true); > > > > > > > > - err = ixgbe_mii_bus_init(hw); > > > > - if (err) > > > > - goto err_netdev; > > > > + ixgbe_mii_bus_init(hw); > > > > > > > > return 0; > > > > > > > > -err_netdev: > > > > - unregister_netdev(netdev); > > > > err_register: > > > > ixgbe_release_hw_control(adapter); > > > > ixgbe_clear_interrupt_scheme(adapter); > > > > -- > > > > 2.14.4 > > > > > > > > > > Then we should check if err == -ENODEV, not outright ignore all > > > potential errors, right? > > > > > > > Hm, it is weird to take -ENODEV as a no error. > > How about just return 0 instead of -ENODEV in the following function? > > > > No, it's perfectly fine. -ENODEV means there's no device and this can > happen. The caller can then act accordingly - for example: ignore that > fact. We do it in several places[1]. > > Bartosz > > [snip] > > [1] > https://elixir.bootlin.com/linux/latest/source/drivers/misc/eeprom/at24.c# > L714 Okay, please go ahead and fix this issue. Thanks, Yongxin
Re: [PATCH v2 3/4] mm: Do early cow for pinned pages during fork() for ptes
On Fri, Sep 25, 2020 at 06:25:59PM -0400, Peter Xu wrote:
> -static inline void
> +/*
> + * Copy one pte. Returns 0 if succeeded, or -EAGAIN if one preallocated page
> + * is required to copy this pte.
> + */
> +static inline int
> copy_present_pte(struct mm_struct *dst_mm, struct mm_struct *src_mm,
> pte_t *dst_pte, pte_t *src_pte, struct vm_area_struct *vma,
> - unsigned long addr, int *rss)
> + struct vm_area_struct *new,
> + unsigned long addr, int *rss, struct page **prealloc)
> {
> unsigned long vm_flags = vma->vm_flags;
> pte_t pte = *src_pte;
> struct page *page;
>
> + page = vm_normal_page(vma, addr, pte);
> + if (page) {
> + if (is_cow_mapping(vm_flags)) {
> + bool is_write = pte_write(pte);
Very minor, but I liked the readability to put this chunk in a
function 'copy_normal_page' with the src/dst naming
> +
> + /*
> + * We have a prealloc page, all good! Take it
> + * over and copy the page & arm it.
> + */
> + *prealloc = NULL;
> + copy_user_highpage(new_page, page, addr, vma);
> + __SetPageUptodate(new_page);
> + pte = mk_pte(new_page, new->vm_page_prot);
> + pte = pte_sw_mkyoung(pte);
Linus's version doesn't do pte_sw_mkyoung(), but looks OK to have it
> + pte = maybe_mkwrite(pte_mkdirty(pte), new);
maybe_mkwrite() was not in Linus's version, but is in
wp_page_copy(). It seemed like mk_pte() should set the proper write
bit already from the vm_page_prot? Perhaps this is harmless but
redundant?
> + page_add_new_anon_rmap(new_page, new, addr,
> false);
> + rss[mm_counter(new_page)]++;
> + set_pte_at(dst_mm, addr, dst_pte, pte);
Linus's patch had a lru_cache_add_inactive_or_unevictable() here, like
wp_page_copy()
Didn't think of anything profound to say, looks good thanks!
I'll forward this for testing as well, there are some holidays next
week so I may have been optimistic to think by Monday.
Jason
Loan Offer
We Offer All Kinds Of Loans, Business Loan, Personal Loan And Much More, If Interested, Contact Us For More Information Via Email: robertfastcapi...@gmail.com or Call Or Text us: +1 407-236-1668. Regards, Robert Reel CEO: Robert Fast Capital Website: robertfastcapital.weebly.com
[rcu:urezki-pcount.2020.09.26a 17/17] kernel/rcu/tree.c:3315:52: sparse: sparse: incorrect type in argument 2 (different base types)
tree: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
urezki-pcount.2020.09.26a
head: e9bed2a1239b017d78cec5de66adce0560f6d077
commit: e9bed2a1239b017d78cec5de66adce0560f6d077 [17/17] kvfree_rcu(): Switch
to kmalloc() and kfree() for allocations
config: i386-randconfig-s002-20200927 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.2-201-g24bdaac6-dirty
#
https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/commit/?id=e9bed2a1239b017d78cec5de66adce0560f6d077
git remote add rcu
https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git
git fetch --no-tags rcu urezki-pcount.2020.09.26a
git checkout e9bed2a1239b017d78cec5de66adce0560f6d077
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
sparse warnings: (new ones prefixed by >>)
kernel/rcu/tree.c:1344:13: sparse: sparse: context imbalance in
'rcu_start_this_gp' - different lock contexts for basic block
kernel/rcu/tree.c:1787:9: sparse: sparse: context imbalance in 'rcu_gp_init'
- different lock contexts for basic block
kernel/rcu/tree.c:2542:9: sparse: sparse: context imbalance in
'force_qs_rnp' - different lock contexts for basic block
kernel/rcu/tree.c:2596:25: sparse: sparse: context imbalance in
'rcu_force_quiescent_state' - unexpected unlock
kernel/rcu/tree.c:3299:29: sparse: sparse: incorrect type in initializer
(different base types) @@ expected int gfp @@ got restricted gfp_t @@
kernel/rcu/tree.c:3299:29: sparse: expected int gfp
kernel/rcu/tree.c:3299:29: sparse: got restricted gfp_t
>> kernel/rcu/tree.c:3315:52: sparse: sparse: incorrect type in argument 2
>> (different base types) @@ expected restricted gfp_t [usertype] flags @@
>>got int gfp @@
>> kernel/rcu/tree.c:3315:52: sparse: expected restricted gfp_t [usertype]
>> flags
kernel/rcu/tree.c:3315:52: sparse: got int gfp
kernel/rcu/tree.c: note: in included file:
kernel/rcu/tree_stall.h:749:17: sparse: sparse: context imbalance in
'rcu_check_gp_start_stall' - different lock contexts for basic block
kernel/rcu/tree.c: note: in included file:
kernel/rcu/tree_exp.h:189:9: sparse: sparse: context imbalance in
'__rcu_report_exp_rnp' - different lock contexts for basic block
vim +3315 kernel/rcu/tree.c
3292
3293 static inline bool
3294 add_ptr_to_bulk_krc_lock(struct kfree_rcu_cpu **krcp,
3295 unsigned long *flags, void *ptr, bool can_sleep)
3296 {
3297 struct kvfree_rcu_bulk_data *bnode;
3298 bool can_alloc_page = preemptible();
3299 int gfp = can_sleep ? GFP_NOWAIT | __GFP_NOWARN : GFP_ATOMIC;
3300 int idx;
3301
3302 *krcp = krc_this_cpu_lock(flags);
3303 if (unlikely(!(*krcp)->initialized))
3304 return false;
3305
3306 idx = !!is_vmalloc_addr(ptr);
3307
3308 /* Check if a new block is required. */
3309 if (!(*krcp)->bkvhead[idx] ||
3310 (*krcp)->bkvhead[idx]->nr_records ==
KVFREE_BULK_MAX_ENTR) {
3311 bnode = get_cached_bnode(*krcp);
3312 if (!bnode && can_alloc_page) {
3313 migrate_disable();
3314 krc_this_cpu_unlock(*krcp, *flags);
> 3315 bnode = kmalloc(PAGE_SIZE, gfp);
3316 *krcp = krc_this_cpu_lock(flags);
3317 migrate_enable();
3318 }
3319
3320 /* Switch to emergency path. */
3321 if (unlikely(!bnode))
3322 return false;
3323
3324 /* Initialize the new block. */
3325 bnode->nr_records = 0;
3326 bnode->next = (*krcp)->bkvhead[idx];
3327
3328 /* Attach it to the head. */
3329 (*krcp)->bkvhead[idx] = bnode;
3330 }
3331
3332 /* Finally insert. */
(*krcp)->bkvhead[idx]->records
3334 [(*krcp)->bkvhead[idx]->nr_records++] = ptr;
3335
3336 return true;
3337 }
3338
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v2 3/7] drivers: hwmon: Add the iEi WT61P803 PUZZLE HWMON driver
Hi Luka,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on lee-mfd/for-mfd-next pavel-linux-leds/for-next
linus/master v5.9-rc6 next-20200925]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Luka-Kovacic/Add-support-for-the-iEi-Puzzle-M801-board/20200926-215756
base:
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
hwmon-next
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/a453994a7710920ee06d179274597737ff37af27
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Luka-Kovacic/Add-support-for-the-iEi-Puzzle-M801-board/20200926-215756
git checkout a453994a7710920ee06d179274597737ff37af27
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=nios2
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/hwmon/iei-wt61p803-puzzle-hwmon.c: In function
'iei_wt61p803_puzzle_hwmon_probe':
>> drivers/hwmon/iei-wt61p803-puzzle-hwmon.c:457:17: warning: variable
>> 'hwmon_dev' set but not used [-Wunused-but-set-variable]
457 | struct device *hwmon_dev;
| ^
vim +/hwmon_dev +457 drivers/hwmon/iei-wt61p803-puzzle-hwmon.c
450
451 static int iei_wt61p803_puzzle_hwmon_probe(struct platform_device *pdev)
452 {
453 struct device *dev = >dev;
454 struct fwnode_handle *child;
455 struct iei_wt61p803_puzzle_hwmon *mcu_hwmon;
456 struct iei_wt61p803_puzzle *mcu = dev_get_drvdata(dev->parent);
> 457 struct device *hwmon_dev;
458 int ret;
459
460 mcu_hwmon = devm_kzalloc(dev, sizeof(*mcu_hwmon), GFP_KERNEL);
461 if (!mcu_hwmon)
462 return -ENOMEM;
463
464 mcu_hwmon->response_buffer = devm_kzalloc(dev,
465 IEI_WT61P803_PUZZLE_BUF_SIZE, GFP_KERNEL);
466 if (!mcu_hwmon->response_buffer)
467 return -ENOMEM;
468
469 mcu_hwmon->mcu = mcu;
470 mutex_init(_hwmon->lock);
471 platform_set_drvdata(pdev, mcu_hwmon);
472
473 hwmon_dev = devm_hwmon_device_register_with_info(dev,
474 "iei_wt61p803_puzzle",
475 mcu_hwmon,
476 _wt61p803_puzzle_chip_info,
477 NULL);
478
479 /* Control fans via PWM lines via Linux Kernel */
480 if (IS_ENABLED(CONFIG_THERMAL)) {
481 device_for_each_child_node(dev, child) {
482 ret =
iei_wt61p803_puzzle_enable_thermal_cooling_dev(dev, child, mcu_hwmon);
483 if (ret) {
484 dev_err(dev, "Enabling the PWM fan
failed\n");
485 fwnode_handle_put(child);
486 return ret;
487 }
488 }
489 }
490 return 0;
491 }
492
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH 1/5] mm: Introduce mm_struct.has_pinned
On Fri, Sep 25, 2020 at 6:15 PM Linus Torvalds wrote: > > I think that over the weekend I'll do Peter's version but with the > "page_mapcount() == 1" check, because I'm starting to like that > better than the mm->has_pinned. Actually, rafter the first read-through, I feel like I'll just apply the series as-is. But I'll look at it some more, and do another read-through and make the final decision tomorrow. If anybody has any concerns about the v2 patch series from Peter, holler. Linus
