Did you get my mail?
-- Hello. I am still waiting to hear from you regarding my proposal. Thanks
RE: [PATCH] compiler.h: Clarify comment about the need for barrier_data()
From: Arvind Sankar > Sent: 15 October 2020 19:14 > > Be clear about @ptr vs the variable that @ptr points to, and add some > more details as to why the special barrier_data() macro is required. > > Signed-off-by: Arvind Sankar > --- > include/linux/compiler.h | 33 ++--- > 1 file changed, 22 insertions(+), 11 deletions(-) > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > index 93035d7fee0d..d8cee7c8968d 100644 > --- a/include/linux/compiler.h > +++ b/include/linux/compiler.h > @@ -86,17 +86,28 @@ void ftrace_likely_update(struct ftrace_likely_data *f, > int val, > > #ifndef barrier_data > /* > - * This version is i.e. to prevent dead stores elimination on @ptr > - * where gcc and llvm may behave differently when otherwise using > - * normal barrier(): while gcc behavior gets along with a normal > - * barrier(), llvm needs an explicit input variable to be assumed > - * clobbered. The issue is as follows: while the inline asm might > - * access any memory it wants, the compiler could have fit all of > - * @ptr into memory registers instead, and since @ptr never escaped > - * from that, it proved that the inline asm wasn't touching any of > - * it. This version works well with both compilers, i.e. we're telling > - * the compiler that the inline asm absolutely may see the contents > - * of @ptr. See also: https://llvm.org/bugs/show_bug.cgi?id=15495 > + * This version is to prevent dead stores elimination on @ptr where gcc and > + * llvm may behave differently when otherwise using normal barrier(): while > gcc > + * behavior gets along with a normal barrier(), llvm needs an explicit input > + * variable to be assumed clobbered. > + * > + * Its primary use is in implementing memzero_explicit(), which is used for > + * clearing temporary data that may contain secrets. > + * > + * The issue is as follows: while the inline asm might access any memory it > + * wants, the compiler could have fit all of the variable that @ptr points to > + * into registers instead, and if @ptr never escaped from the function, it > + * proved that the inline asm wasn't touching any of it. gcc only eliminates > + * dead stores if the variable was actually allocated in registers, but llvm > + * reasons that the variable _could_ have been in registers, so the inline > asm > + * can't reliably access it anyway, and eliminates dead stores even if the > + * variable is actually in memory. I think I'd just say something like: Although the compiler must assume a "memory" clobber may affect all memory, local variables (on stack) cannot actually be visible to the asm unless their address has been passed to an external function. So the compiler may assume such variables cannot be affected by a normal asm volatile(::"memory") barrier(). Passing the address of the local variables to the asm barrier is enough to tell the compiler that the asm can 'see' the variables (and spill anything held in registers to the stack) so that the "memory" clobber has the expected effect. This is necessary to get llvm to do a memset() of on-stack data at the end of a function to clear memory that contains secrets. David > + * > + * This version works well with both compilers, i.e. we're telling the > compiler > + * that the inline asm absolutely may see the contents of the variable > pointed > + * to by @ptr. > + * > + * See also: https://llvm.org/bugs/show_bug.cgi?id=15495#c5 > */ > # define barrier_data(ptr) __asm__ __volatile__("": :"r"(ptr) :"memory") > #endif > -- > 2.26.2 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On Thu, 2020-10-15 at 19:24 +0530, Bhaskar Chowdhury wrote: > On 06:38 Thu 15 Oct 2020, Joe Perches wrote: > > On Thu, 2020-10-15 at 18:53 +0530, Bhaskar Chowdhury wrote: > > > Fix the space in the middle in below entry. > > > > > > memry||memory > > [] > > > diff --git a/scripts/spelling.txt b/scripts/spelling.txt > > [] > > > @@ -885,7 +885,7 @@ meetign||meeting > > > memeory||memory > > > memmber||member > > > memoery||memory > > > -memry ||memory > > > +memry||memory > > > > No. Don't post a bad patch, assume > > it's applied and then post a fix to > > the bad patch as v2. > > > > Send a single clean patch. > > > > Not sure what you mean...could you elaborate...don't know what is going on..> You sent a patch with a defect You sent a V2 patch that just corrects the defect in the first patch. Instead send a v3 patch without any defect.
Re: linux-next: build failure after merge of the hmm tree
Hi all, On Mon, 12 Oct 2020 15:19:48 +1100 Stephen Rothwell wrote: > > On Tue, 6 Oct 2020 13:41:20 -0300 Jason Gunthorpe wrote: > > > > On Tue, Oct 06, 2020 at 08:35:08PM +1100, Stephen Rothwell wrote: > > > Hi all, > > > > > > After merging the hmm tree, today's linux-next build (arm > > > multi_v7_defconfig) failed like this: > > > > > > > > > Caused by commit > > > > > > 07da1223ec93 ("lib/scatterlist: Add support in dynamic allocation of SG > > > table from pages") > > > > > > interacting with commit > > > > > > 707d561f77b5 ("drm: allow limiting the scatter list size.") > > > > > > from the drm tree. > > > > > > I have added the following merge fix patch > > > > > > From: Stephen Rothwell > > > Date: Tue, 6 Oct 2020 20:22:51 +1100 > > > Subject: [PATCH] lib/scatterlist: merge fix for "drm: allow limiting the > > > scatter list size." > > > > > > Signed-off-by: Stephen Rothwell > > > drivers/gpu/drm/drm_prime.c | 9 ++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > > > index 11fe9ff76fd5..83ac901b65a2 100644 > > > +++ b/drivers/gpu/drm/drm_prime.c > > > @@ -807,6 +807,7 @@ struct sg_table *drm_prime_pages_to_sg(struct > > > drm_device *dev, > > > struct page **pages, unsigned int > > > nr_pages) > > > { > > > struct sg_table *sg = NULL; > > > + struct scatterlist *sl; > > > size_t max_segment = 0; > > > int ret; > > > > > > @@ -820,11 +821,13 @@ struct sg_table *drm_prime_pages_to_sg(struct > > > drm_device *dev, > > > max_segment = dma_max_mapping_size(dev->dev); > > > if (max_segment == 0 || max_segment > SCATTERLIST_MAX_SEGMENT) > > > max_segment = SCATTERLIST_MAX_SEGMENT; > > > - ret = __sg_alloc_table_from_pages(sg, pages, nr_pages, 0, > > > + sl = __sg_alloc_table_from_pages(sg, pages, nr_pages, 0, > > > nr_pages << PAGE_SHIFT, > > > - max_segment, GFP_KERNEL); > > > - if (ret) > > > + max_segment, NULL, 0, GFP_KERNEL); > > > + if (IS_ERR(sl)) { > > > + ret = PTR_ERR(sl); > > > goto out; > > > + } > > > > > > return sg; > > > out: > > > > This looks OK to me, thanks > > This merge fix patch is now being applied to the merge of the drm tree > since the rdma tree (that is merged before the drm tree) has merged the > hmm tree. This merge fix patch is now being applied to the merge of the rdma tree since the Linus has merged the drm tree. -- Cheers, Stephen Rothwell pgpLAtQQAbSSV.pgp Description: OpenPGP digital signature
[v4.9..v5.4/bluetooth PATCH 3/3] Bluetooth: Disconnect if E0 is used for Level 4
From: Luiz Augusto von Dentz E0 is not allowed with Level 4: BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319: '128-bit equivalent strength for link and encryption keys required using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, and P-192 not allowed; encryption key not shortened' SC enabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) Secure Connections (Host Support) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with AES-CCM (0x02) SC disabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with E0 (0x01) [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used < HCI Command: Disconnect (0x01|0x0006) plen 3 Handle: 256 Reason: Authentication Failure (0x05) Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 8746f135bb01872ff412d408ea1aa9ebd328c1f5) Cc: sta...@vger.kernel.org # 4.9..5.4 --- include/net/bluetooth/hci_core.h | 10 ++ net/bluetooth/hci_conn.c | 17 + net/bluetooth/hci_event.c| 20 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 144f580556f9..3cd232cf29c6 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1329,11 +1329,13 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) else encrypt = 0x01; - if (conn->sec_level == BT_SECURITY_SDP) - conn->sec_level = BT_SECURITY_LOW; + if (!status) { + if (conn->sec_level == BT_SECURITY_SDP) + conn->sec_level = BT_SECURITY_LOW; - if (conn->pending_sec_level > conn->sec_level) - conn->sec_level = conn->pending_sec_level; + if (conn->pending_sec_level > conn->sec_level) + conn->sec_level = conn->pending_sec_level; + } mutex_lock(&hci_cb_list_lock); list_for_each_entry(cb, &hci_cb_list, list) { diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 87691404d0c6..ee57fa20bac3 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1285,6 +1285,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) return 0; } +/* AES encryption is required for Level 4: + * + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C + * page 1319: + * + * 128-bit equivalent strength for link and encryption keys + * required using FIPS approved algorithms (E0 not allowed, + * SAFER+ not allowed, and P-192 not allowed; encryption key + * not shortened) + */ + if (conn->sec_level == BT_SECURITY_FIPS && + !test_bit(HCI_CONN_AES_CCM, &conn->flags)) { + bt_dev_err(conn->hdev, + "Invalid security: Missing AES-CCM usage"); + return 0; + } + if (hci_conn_ssp_enabled(conn) && !test_bit(HCI_CONN_ENCRYPT, &conn->flags)) return 0; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index de4cce5f1bd8..9917b399ddd0 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2974,27 +2974,23 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags); + /* Check link security requirements are met */ + if (!hci_conn_check_link_mode(conn)) + ev->status = HCI_ERROR_AUTH_FAILURE; + if (ev->status && conn->state == BT_CONNECTED) { if (ev->status == HCI_ERROR_PIN_OR_KEY_MISSING) set_bit(HCI_CONN_AUTH_FAILURE, &conn->flags); + /* Notify upper layers so they can cleanup before +* disconnecting. +*/ + hci_encrypt_cfm(conn, ev->status); hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); hci_conn_drop(conn); goto unlock; } - /* In Secure Connections Only mode, do not allow any connections -* that are not encrypted with AES-CCM using a P-256 authenticated -* combination key. -*/ -
[v5.8/bluetooth PATCH] Bluetooth: Disconnect if E0 is used for Level 4
From: Luiz Augusto von Dentz E0 is not allowed with Level 4: BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319: '128-bit equivalent strength for link and encryption keys required using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, and P-192 not allowed; encryption key not shortened' SC enabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) Secure Connections (Host Support) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with AES-CCM (0x02) SC disabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with E0 (0x01) [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used < HCI Command: Disconnect (0x01|0x0006) plen 3 Handle: 256 Reason: Authentication Failure (0x05) Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 8746f135bb01872ff412d408ea1aa9ebd328c1f5) Cc: sta...@vger.kernel.org # 5.8 --- include/net/bluetooth/hci_core.h | 10 ++ net/bluetooth/hci_conn.c | 17 + net/bluetooth/hci_event.c| 20 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index da3728871e85..78970afa96f9 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1402,11 +1402,13 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) else encrypt = 0x01; - if (conn->sec_level == BT_SECURITY_SDP) - conn->sec_level = BT_SECURITY_LOW; + if (!status) { + if (conn->sec_level == BT_SECURITY_SDP) + conn->sec_level = BT_SECURITY_LOW; - if (conn->pending_sec_level > conn->sec_level) - conn->sec_level = conn->pending_sec_level; + if (conn->pending_sec_level > conn->sec_level) + conn->sec_level = conn->pending_sec_level; + } mutex_lock(&hci_cb_list_lock); list_for_each_entry(cb, &hci_cb_list, list) { diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 307800fd18e6..b99b5c6de55a 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1323,6 +1323,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) return 0; } +/* AES encryption is required for Level 4: + * + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C + * page 1319: + * + * 128-bit equivalent strength for link and encryption keys + * required using FIPS approved algorithms (E0 not allowed, + * SAFER+ not allowed, and P-192 not allowed; encryption key + * not shortened) + */ + if (conn->sec_level == BT_SECURITY_FIPS && + !test_bit(HCI_CONN_AES_CCM, &conn->flags)) { + bt_dev_err(conn->hdev, + "Invalid security: Missing AES-CCM usage"); + return 0; + } + if (hci_conn_ssp_enabled(conn) && !test_bit(HCI_CONN_ENCRYPT, &conn->flags)) return 0; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 6c6c9a81bee2..ff38f98988db 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3068,27 +3068,23 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags); + /* Check link security requirements are met */ + if (!hci_conn_check_link_mode(conn)) + ev->status = HCI_ERROR_AUTH_FAILURE; + if (ev->status && conn->state == BT_CONNECTED) { if (ev->status == HCI_ERROR_PIN_OR_KEY_MISSING) set_bit(HCI_CONN_AUTH_FAILURE, &conn->flags); + /* Notify upper layers so they can cleanup before +* disconnecting. +*/ + hci_encrypt_cfm(conn, ev->status); hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); hci_conn_drop(conn); goto unlock; } - /* In Secure Connections Only mode, do not allow any connections -* that are not encrypted with AES-CCM using a P-256 authenticated -* combination key. -*/ -
[v4.9..v5.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm`
From: Patrick Steinhardt Starting with the upgrade to v5.8-rc3, I've noticed I wasn't able to connect to my Bluetooth headset properly anymore. While connecting to the device would eventually succeed, bluetoothd seemed to be confused about the current connection state where the state was flapping hence and forth. Bisecting this issue led to commit 3ca44c16b0dc (Bluetooth: Consolidate encryption handling in hci_encrypt_cfm, 2020-05-19), which refactored `hci_encrypt_cfm` to also handle updating the connection state. The commit in question changed the code to call `hci_connect_cfm` inside `hci_encrypt_cfm` and to change the connection state. But with the conversion, we now only update the connection state if a status was set already. In fact, the reverse should be true: the status should be updated if no status is yet set. So let's fix the isuse by reversing the condition. Fixes: 3ca44c16b0dc ("Bluetooth: Consolidate encryption handling in hci_encrypt_cfm") Signed-off-by: Patrick Steinhardt Acked-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 339ddaa626995bc6218972ca241471f3717cc5f4) Cc: sta...@vger.kernel.org # 4.9..5.4 --- include/net/bluetooth/hci_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index ffd0eedd27ab..144f580556f9 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1314,7 +1314,7 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) __u8 encrypt; if (conn->state == BT_CONFIG) { - if (status) + if (!status) conn->state = BT_CONNECTED; hci_connect_cfm(conn, status); -- 2.27.0
[v4.4/bluetooth PATCH 2/3] Bluetooth: Fix update of connection state in `hci_encrypt_cfm`
From: Patrick Steinhardt Starting with the upgrade to v5.8-rc3, I've noticed I wasn't able to connect to my Bluetooth headset properly anymore. While connecting to the device would eventually succeed, bluetoothd seemed to be confused about the current connection state where the state was flapping hence and forth. Bisecting this issue led to commit 3ca44c16b0dc (Bluetooth: Consolidate encryption handling in hci_encrypt_cfm, 2020-05-19), which refactored `hci_encrypt_cfm` to also handle updating the connection state. The commit in question changed the code to call `hci_connect_cfm` inside `hci_encrypt_cfm` and to change the connection state. But with the conversion, we now only update the connection state if a status was set already. In fact, the reverse should be true: the status should be updated if no status is yet set. So let's fix the isuse by reversing the condition. Fixes: 3ca44c16b0dc ("Bluetooth: Consolidate encryption handling in hci_encrypt_cfm") Signed-off-by: Patrick Steinhardt Acked-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 339ddaa626995bc6218972ca241471f3717cc5f4) Cc: sta...@vger.kernel.org # 4.4 --- include/net/bluetooth/hci_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 0269a772bfe1..dfa672b6f89d 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1241,7 +1241,7 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) __u8 encrypt; if (conn->state == BT_CONFIG) { - if (status) + if (!status) conn->state = BT_CONNECTED; hci_connect_cfm(conn, status); -- 2.27.0
[v4.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
From: Luiz Augusto von Dentz This makes hci_encrypt_cfm calls hci_connect_cfm in case the connection state is BT_CONFIG so callers don't have to check the state. Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 3ca44c16b0dcc764b641ee4ac226909f5c421aa3) Cc: sta...@vger.kernel.org # 4.4 --- include/net/bluetooth/hci_core.h | 20 ++-- net/bluetooth/hci_event.c| 28 +++- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 7c0c83dfe86e..0269a772bfe1 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1235,10 +1235,26 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) conn->security_cfm_cb(conn, status); } -static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status, - __u8 encrypt) +static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) { struct hci_cb *cb; + __u8 encrypt; + + if (conn->state == BT_CONFIG) { + if (status) + conn->state = BT_CONNECTED; + + hci_connect_cfm(conn, status); + hci_conn_drop(conn); + return; + } + + if (!test_bit(HCI_CONN_ENCRYPT, &conn->flags)) + encrypt = 0x00; + else if (test_bit(HCI_CONN_AES_CCM, &conn->flags)) + encrypt = 0x02; + else + encrypt = 0x01; if (conn->sec_level == BT_SECURITY_SDP) conn->sec_level = BT_SECURITY_LOW; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 03319ab8a7c6..bb9c13506bca 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2479,7 +2479,7 @@ static void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) &cp); } else { clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags); - hci_encrypt_cfm(conn, ev->status, 0x00); + hci_encrypt_cfm(conn, ev->status); } } @@ -2565,22 +2565,7 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status, conn->enc_key_size = rp->key_size; } - if (conn->state == BT_CONFIG) { - conn->state = BT_CONNECTED; - hci_connect_cfm(conn, 0); - hci_conn_drop(conn); - } else { - u8 encrypt; - - if (!test_bit(HCI_CONN_ENCRYPT, &conn->flags)) - encrypt = 0x00; - else if (test_bit(HCI_CONN_AES_CCM, &conn->flags)) - encrypt = 0x02; - else - encrypt = 0x01; - - hci_encrypt_cfm(conn, 0, encrypt); - } + hci_encrypt_cfm(conn, 0); unlock: hci_dev_unlock(hdev); @@ -2674,14 +2659,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) } notify: - if (conn->state == BT_CONFIG) { - if (!ev->status) - conn->state = BT_CONNECTED; - - hci_connect_cfm(conn, ev->status); - hci_conn_drop(conn); - } else - hci_encrypt_cfm(conn, ev->status, ev->encrypt); + hci_encrypt_cfm(conn, ev->status); unlock: hci_dev_unlock(hdev); -- 2.27.0
[GIT PULL] Kunit fixes update for Linux 5.10-rc1
Hi Linus, Please pull the following Kunit fixes update for Linux 5.10-rc1 This Kunit fixes update consists of several kunit tool bug fixes in flag handling, run outside kernel tree, make errors, and generating results. diff is attached. thanks, -- Shuah The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5: Linux 5.9-rc1 (2020-08-16 13:04:57 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest tags/linux-kselftest-kunit-fixes-5.10-rc1 for you to fetch changes up to 1abdd39f14b25dd2d69096b624a4f86f158a9feb: kunit: tool: fix display of make errors (2020-10-09 14:04:09 -0600) linux-kselftest-kunit-fixes-5.10-rc1 This Kunit fixes update consists of several kunit tool bug fixes in flag handling, run outside kernel tree, make errors, and generating results. Brendan Higgins (3): kunit: tool: fix running kunit_tool from outside kernel tree kunit: tool: fix --alltests flag kunit: tool: handle when .kunit exists but .kunitconfig does not Daniel Latypov (1): kunit: tool: fix display of make errors Heidi Fahim (1): kunit: tool: allow generating test results in JSON tools/testing/kunit/configs/broken_on_uml.config | 1 + tools/testing/kunit/kunit.py | 58 +++--- tools/testing/kunit/kunit_json.py| 63 tools/testing/kunit/kunit_kernel.py | 27 +- tools/testing/kunit/kunit_tool_test.py | 33 + 5 files changed, 154 insertions(+), 28 deletions(-) create mode 100644 tools/testing/kunit/kunit_json.py diff --git a/tools/testing/kunit/configs/broken_on_uml.config b/tools/testing/kunit/configs/broken_on_uml.config index 239b9f03da2c..a7f0603d33f6 100644 --- a/tools/testing/kunit/configs/broken_on_uml.config +++ b/tools/testing/kunit/configs/broken_on_uml.config @@ -39,3 +39,4 @@ # CONFIG_QCOM_CPR is not set # CONFIG_RESET_BRCMSTB_RESCAL is not set # CONFIG_RESET_INTEL_GW is not set +# CONFIG_ADI_AXI_ADC is not set diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py index 425ef40067e7..ebf5f5763dee 100755 --- a/tools/testing/kunit/kunit.py +++ b/tools/testing/kunit/kunit.py @@ -17,6 +17,7 @@ from collections import namedtuple from enum import Enum, auto import kunit_config +import kunit_json import kunit_kernel import kunit_parser @@ -30,9 +31,9 @@ KunitBuildRequest = namedtuple('KunitBuildRequest', KunitExecRequest = namedtuple('KunitExecRequest', ['timeout', 'build_dir', 'alltests']) KunitParseRequest = namedtuple('KunitParseRequest', - ['raw_output', 'input_data']) + ['raw_output', 'input_data', 'build_dir', 'json']) KunitRequest = namedtuple('KunitRequest', ['raw_output','timeout', 'jobs', - 'build_dir', 'alltests', + 'build_dir', 'alltests', 'json', 'make_options']) KernelDirectoryPath = sys.argv[0].split('tools/testing/kunit/')[0] @@ -113,12 +114,22 @@ def parse_tests(request: KunitParseRequest) -> KunitResult: test_result = kunit_parser.TestResult(kunit_parser.TestStatus.SUCCESS, [], 'Tests not Parsed.') + if request.raw_output: kunit_parser.raw_output(request.input_data) else: test_result = kunit_parser.parse_run_tests(request.input_data) parse_end = time.time() + if request.json: + json_obj = kunit_json.get_json_result( + test_result=test_result, + def_config='kunit_defconfig', + build_dir=request.build_dir, + json_path=request.json) + if request.json == 'stdout': + print(json_obj) + if test_result.status != kunit_parser.TestStatus.SUCCESS: return KunitResult(KunitStatus.TEST_FAILURE, test_result, parse_end - parse_start) @@ -151,7 +162,9 @@ def run_tests(linux: kunit_kernel.LinuxSourceTree, return exec_result parse_request = KunitParseRequest(request.raw_output, - exec_result.result) + exec_result.result, + request.build_dir, + request.json) parse_result = parse_tests(parse_request) run_end = time.time() @@ -195,7 +208,12 @@ def add_exec_opts(parser): def add_parse_opts(parser): parser.add_argument('--raw_output', help='don\'t format output from kernel', action='store_true') - + parser.add_argument('--json', + nargs='?', + help='Stores test results in a JSON, and either ' + 'prints to stdout or saves to file if a ' + 'filename is specified', + type=str, const='stdout', default=None) def main(argv, linux=None): parser = argparse.ArgumentParser( @@ -237,10 +255,16 @@ def main(argv, linux=None): cli_args = parser.parse_args(argv) + if get_kernel_root_path(): +
[v4.4/bluetooth PATCH 3/3] Bluetooth: Disconnect if E0 is used for Level 4
From: Luiz Augusto von Dentz E0 is not allowed with Level 4: BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C page 1319: '128-bit equivalent strength for link and encryption keys required using FIPS approved algorithms (E0 not allowed, SAFER+ not allowed, and P-192 not allowed; encryption key not shortened' SC enabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x0b 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) Secure Connections (Host Support) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with AES-CCM (0x02) SC disabled: > HCI Event: Read Remote Extended Features (0x23) plen 13 Status: Success (0x00) Handle: 256 Page: 1/2 Features: 0x03 0x00 0x00 0x00 0x00 0x00 0x00 0x00 Secure Simple Pairing (Host Support) LE Supported (Host) > HCI Event: Encryption Change (0x08) plen 4 Status: Success (0x00) Handle: 256 Encryption: Enabled with E0 (0x01) [May 8 20:23] Bluetooth: hci0: Invalid security: expect AES but E0 was used < HCI Command: Disconnect (0x01|0x0006) plen 3 Handle: 256 Reason: Authentication Failure (0x05) Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 8746f135bb01872ff412d408ea1aa9ebd328c1f5, adjusted to match linux-4.4.y sources.) Cc: sta...@vger.kernel.org # 4.4 --- include/net/bluetooth/hci_core.h | 10 ++ net/bluetooth/hci_conn.c | 17 + net/bluetooth/hci_event.c| 20 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index dfa672b6f89d..5aaf6cdb121a 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1256,11 +1256,13 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) else encrypt = 0x01; - if (conn->sec_level == BT_SECURITY_SDP) - conn->sec_level = BT_SECURITY_LOW; + if (!status) { + if (conn->sec_level == BT_SECURITY_SDP) + conn->sec_level = BT_SECURITY_LOW; - if (conn->pending_sec_level > conn->sec_level) - conn->sec_level = conn->pending_sec_level; + if (conn->pending_sec_level > conn->sec_level) + conn->sec_level = conn->pending_sec_level; + } mutex_lock(&hci_cb_list_lock); list_for_each_entry(cb, &hci_cb_list, list) { diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 114bcf6ea916..2c94e3cd3545 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1173,6 +1173,23 @@ int hci_conn_check_link_mode(struct hci_conn *conn) return 0; } +/* AES encryption is required for Level 4: + * + * BLUETOOTH CORE SPECIFICATION Version 5.2 | Vol 3, Part C + * page 1319: + * + * 128-bit equivalent strength for link and encryption keys + * required using FIPS approved algorithms (E0 not allowed, + * SAFER+ not allowed, and P-192 not allowed; encryption key + * not shortened) + */ + if (conn->sec_level == BT_SECURITY_FIPS && + !test_bit(HCI_CONN_AES_CCM, &conn->flags)) { + bt_dev_err(conn->hdev, + "Invalid security: Missing AES-CCM usage"); + return 0; + } + if (hci_conn_ssp_enabled(conn) && !test_bit(HCI_CONN_ENCRYPT, &conn->flags)) return 0; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index bb9c13506bca..f0e6cce921d8 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2612,24 +2612,20 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags); + /* Check link security requirements are met */ + if (!hci_conn_check_link_mode(conn)) + ev->status = HCI_ERROR_AUTH_FAILURE; + if (ev->status && conn->state == BT_CONNECTED) { + /* Notify upper layers so they can cleanup before +* disconnecting. +*/ + hci_encrypt_cfm(conn, ev->status); hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); hci_conn_drop(conn); goto unlock; } - /* In Secure Connections Only mode, do not allow any connections -* that are not encrypted with AES-CCM using a P-256 authenticated -* combination key. -*/ - if (hci_dev_test_flag(hdev, HCI_SC_ONLY) && - (!test_bit(HCI_CONN_AES_CCM, &conn->fl
[v4.9..v5.4/bluetooth PATCH 1/3] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
From: Luiz Augusto von Dentz This makes hci_encrypt_cfm calls hci_connect_cfm in case the connection state is BT_CONFIG so callers don't have to check the state. Signed-off-by: Luiz Augusto von Dentz Signed-off-by: Marcel Holtmann (cherry picked from commit 3ca44c16b0dcc764b641ee4ac226909f5c421aa3) Cc: sta...@vger.kernel.org # 4.9..5.4 --- include/net/bluetooth/hci_core.h | 20 ++-- net/bluetooth/hci_event.c| 28 +++- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index b689aceb636b..ffd0eedd27ab 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1308,10 +1308,26 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status) conn->security_cfm_cb(conn, status); } -static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status, - __u8 encrypt) +static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status) { struct hci_cb *cb; + __u8 encrypt; + + if (conn->state == BT_CONFIG) { + if (status) + conn->state = BT_CONNECTED; + + hci_connect_cfm(conn, status); + hci_conn_drop(conn); + return; + } + + if (!test_bit(HCI_CONN_ENCRYPT, &conn->flags)) + encrypt = 0x00; + else if (test_bit(HCI_CONN_AES_CCM, &conn->flags)) + encrypt = 0x02; + else + encrypt = 0x01; if (conn->sec_level == BT_SECURITY_SDP) conn->sec_level = BT_SECURITY_LOW; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index fd436e5d7b54..de4cce5f1bd8 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2840,7 +2840,7 @@ static void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) &cp); } else { clear_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags); - hci_encrypt_cfm(conn, ev->status, 0x00); + hci_encrypt_cfm(conn, ev->status); } } @@ -2925,22 +2925,7 @@ static void read_enc_key_size_complete(struct hci_dev *hdev, u8 status, conn->enc_key_size = rp->key_size; } - if (conn->state == BT_CONFIG) { - conn->state = BT_CONNECTED; - hci_connect_cfm(conn, 0); - hci_conn_drop(conn); - } else { - u8 encrypt; - - if (!test_bit(HCI_CONN_ENCRYPT, &conn->flags)) - encrypt = 0x00; - else if (test_bit(HCI_CONN_AES_CCM, &conn->flags)) - encrypt = 0x02; - else - encrypt = 0x01; - - hci_encrypt_cfm(conn, 0, encrypt); - } + hci_encrypt_cfm(conn, 0); unlock: hci_dev_unlock(hdev); @@ -3058,14 +3043,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) } notify: - if (conn->state == BT_CONFIG) { - if (!ev->status) - conn->state = BT_CONNECTED; - - hci_connect_cfm(conn, ev->status); - hci_conn_drop(conn); - } else - hci_encrypt_cfm(conn, ev->status, ev->encrypt); + hci_encrypt_cfm(conn, ev->status); unlock: hci_dev_unlock(hdev); -- 2.27.0
Re: [PATCH] power: reset: POWER_RESET_OCELOT_RESET should depend on Ocelot or Sparx5
Hi, On Wed, Oct 14, 2020 at 03:14:15PM +0200, Geert Uytterhoeven wrote: > To add support for Sparx5, the dependency on MSCC_OCELOT was removed. > However, this increases exposure of the driver question not only to > Sparx5 platforms, but to everyone. Hence re-add the dependency on > MSCC_OCELOT, and extend it with ARCH_SPARX5, to prevent asking the user > about this driver when configuring a kernel without Ocelot and Sparx5 > support. > > Fixes: ec871696b7776767 ("power: reset: ocelot: Add support for Sparx5") > Signed-off-by: Geert Uytterhoeven Thanks, queued. -- Sebastian > drivers/power/reset/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig > index 6361569aacb7eedf..d55b3727e00eb768 100644 > --- a/drivers/power/reset/Kconfig > +++ b/drivers/power/reset/Kconfig > @@ -129,6 +129,7 @@ config POWER_RESET_QCOM_PON > > config POWER_RESET_OCELOT_RESET > bool "Microsemi Ocelot reset driver" > + depends on MSCC_OCELOT || ARCH_SPARX5 || COMPILE_TEST > select MFD_SYSCON > help > This driver supports restart for Microsemi Ocelot SoC and similar. > -- > 2.17.1 > signature.asc Description: PGP signature
Re: [PATCH v4.4/bluetooth 1/2] Bluetooth: Consolidate encryption handling in hci_encrypt_cfm
On 15/10/2020 14:44, Hans-Christian Egtvedt (hegtvedt) wrote: > On 15/10/2020 14:02, Greg KH wrote: >> On Thu, Oct 15, 2020 at 11:18:39AM +, Hans-Christian Egtvedt (hegtvedt) >> wrote: >>> On 15/10/2020 11:57, Greg KH wrote: On Thu, Oct 15, 2020 at 09:43:32AM +0200, Hans-Christian Noren Egtvedt wrote: > From: Luiz Augusto von Dentz > --- > AFAICT, fixing CVE 2020-10135 Bluetooth impersonation attacks have been > left out for the 4.4 stable kernel. I cherry picked what I assume are > the appropriate two patches missing from the 4.9 stable kernel. Please > add them to upcoming 4.4 stable releases. Why are you merging 2 commits together? Please provide backports for all stable kernels, if you want to see this in the 4.4.y tree. We can not have someone move from an older tree to a newer one and have a regression. >>> >>> Agreed, I have managed to trick myself into thinking the 4.4.y branch >>> was left out, but I assume these patches are required for all LTS branches. >> >> They are, but if you have copies of them, please feel free to share >> them. > > I will repeat cherry-picking from a clean linux-stable git tree and send > patches, sorry for this noise. > > I see linux-5.8.y has partial patches, while the older branches need the > full series of three commits. I just discovered an additional fix. I sent three iterations. 5.8.y only requires a single cherry-pick. 4.9.y to 5.4.y has non-conflicting cherry-pick from upstream commits. 4.4.y needs to resolve a conflict when cherry-picking. I would still want Luiz to ack that this completes the mitigation for this Bluetooth vulnerability in the stable kernels. -- Best regards, Hans-Christian Noren Egtvedt pEpkey.asc Description: pEpkey.asc
Re: [PATCH 0/3] IRQ stack support for ARM
On 10/15/20 1:59 PM, Nick Desaulniers wrote: > On Thu, Oct 8, 2020 at 1:30 AM Russell King - ARM Linux admin > wrote: >> >> On Thu, Oct 08, 2020 at 12:45:30PM +0530, Maninder Singh wrote: >>> Observed Stack Overflow on 8KB kernel stack on ARM specially >>> incase on network interrupts, which results in undeterministic behaviour. >>> So there is need for per cpu dedicated IRQ stack for ARM. >>> >>> As ARm does not have extra co-processor register >>> to save thread info pointer, IRQ stack will be at some >>> performance cost, so code is under CONFIG_IRQ_STACK. >>> >>> and we don't have much knowledge and set up for CLANG >>> and ARM_UNWIND, so dependency added for both cases. >>> >>> Tested patch set with QEMU for latest kernel >>> and 4.1 kernel for ARM target with same patch set. >> >> You need to investigate and show where and why this is happening. My >> guess is you have a network driver that uses a lot of kernel stack >> space, which itself would be a bug. >> >> Note that there are compiler versions out there that mis-optimise and >> eat stack space - the kernel build should be warning if a function >> uses a large amount of stack. > > For tracking down those not-super-helpful compiler warnings, I wrote a > tool where if you rebuild with debug info, and give it the object file > and string of the function the compiler warned about it will parse the > DWARF to tell you the size of each local variable, and if it came from > an inline frame. Generally, it's possible to stack allocate something > that's way too big; instead those should be allocated on the heap. > https://github.com/ClangBuiltLinux/frame-larger-than > (I haven't had time to sit down and use it to resolve all outstanding > issues, but it has worked well for me in the past) Things get a bit more difficult with the network stack and you easily recurse into functions and blow up the stack size. This is especially true if you have some complex network tunneling or filtering going on. For one, in the 4.1 kernel that appears to have been used as a basis for this work, if you have CONFIG_BPF enabled but not CONFIG_BPF_JIT_ALWAYS_ON, __bpf_prog_run will require about 724 bytes of stack last I measured, that's nearly 10% of the stack that goes away just like that. -- Florian
Re: [PATCH 5/8] x86/clear_page: add clear_page_uncached()
On 2020-10-15 3:35 a.m., Borislav Petkov wrote: On Wed, Oct 14, 2020 at 08:37:44PM -0700, Ankur Arora wrote: I don't disagree but I think the selection of cached/uncached route should be made where we have enough context available to be able to choose to do this. This could be for example, done in mm_populate() or gup where if say the extent is larger than LLC-size, it takes the uncached path. Are there examples where we don't know the size? The case I was thinking of was that clear_huge_page() or faultin_page() would know the size to a page unit, while the higher level function would know the whole extent and could optimize differently based on that. Thanks Ankur
Re: [PATCH net 1/4] ptp: ptp_idt82p33: update to support adjphase
On Thu, Oct 15, 2020 at 07:30:38PM +, Min Li wrote: > When you have time, can you take a look at this change? Thanks Min, I think your series was posted during a time when net-next was closed. Please report the series. Thanks, Richard
Re: [PATCH 2/2] irqchip/sifive-plic: Fix the interrupt was enabled accidentally issue.
On 2020-10-12 14:57, Greentime Hu wrote: In commit 2ca0b460bbcb ("genirq/affinity: Make affinity setting if activated opt-in"), it added irqd_affinity_on_activate() checking in the function irq_set_affinity_deactivated() so it will return false here. In that case, it will call irq_try_set_affinity() -> plic_irq_toggle() which will enable the interrupt to cause the CPU hang. if (irq_set_affinity_deactivated()) return 0; ... irq_try_set_affinity(data, mask, force); [ 919.015783] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: [ 919.020922] rcu: 0-...0: (0 ticks this GP) idle=7d2/1/0x4002 softirq=1424/1424 fqs=105807 [ 919.030295] (detected by 1, t=225825 jiffies, g=1561, q=3496) [ 919.036109] Task dump for CPU 0: [ 919.039321] kworker/0:1 R running task030 2 0x0008 [ 919.046359] Workqueue: events set_brightness_delayed [ 919.051302] Call Trace: [ 919.053738] [] __schedule+0x194/0x4de [ 982.035783] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: [ 982.040923] rcu: 0-...0: (0 ticks this GP) idle=7d2/1/0x4002 softirq=1424/1424 fqs=113325 [ 982.050294] (detected by 1, t=241580 jiffies, g=1561, q=3509) [ 982.056108] Task dump for CPU 0: [ 982.059321] kworker/0:1 R running task030 2 0x0008 [ 982.066359] Workqueue: events set_brightness_delayed [ 982.071302] Call Trace: [ 982.073739] [] __schedule+0x194/0x4de [..] After applying this patch, the CPU hang issue can be fixed. Signed-off-by: Greentime Hu --- drivers/irqchip/irq-sifive-plic.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 4cc8a2657a6d..8a673d9cff69 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -183,10 +183,14 @@ static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) { struct plic_priv *priv = d->host_data; + struct irq_data *irqd; irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data, handle_fasteoi_irq, NULL, NULL); irq_set_noprobe(irq); + irqd = irq_get_irq_data(irq); + irqd_set_single_target(irqd); + irqd_set_affinity_on_activate(irqd); irq_set_affinity(irq, &priv->lmask); return 0; } How does this fix anything? The plic driver doesn't have an activate callback, so how does it make any difference? I get the feeling this papers over another issue. M. -- Jazz is not dead. It just smells funny...
Re: [PATCH v6 35/80] docs: fs: fscrypt.rst: get rid of :c:type: tags
On Thu, 15 Oct 2020 07:32:07 +0200 Mauro Carvalho Chehab wrote: > > That will apply to most (maybe all) of the structures mentioned in this > > file. > > I expected that if the documentation system now automatically recognizes > > 'struct foo', then it would render it in code font even when 'struct foo' > > isn't > > documented. Any particular reason why that isn't the case? Not like I care > > much myself, but it's a bit unexpected and it means this change actually > > makes > > the rendered documentation look worse... > > Yeah, I agree that using monospaced fonts on this case too would > be nice. The C domain actually uses italic monospaced fonts for > broken XREFs. > > I suspect that changing this at automarkup.py would be simple, but > not sure if it would be safe. > > Jon can tell more about that, as he's the author of automarkup, > but I suspect that the reason for the current behavior is to avoid > false-positives. Automarkup has always behaved that way because ... well, because nobody got around to changing it. I don't see any reason not to use a monospace font for such things, just without a link; shouldn't be a problem to do. I'll see if I can't get to it once things stabilize a bit. Thanks, jon
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: 93b694d096cc10994c817730d4d50288f9ae3d66 commit: cd33c830448baf7b1e94da72eca069e3e1d050c9 media: rkvdec: Add the rkvdec driver date: 6 months ago config: x86_64-randconfig-a001-20201016 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project e7b4feea8e1bf520b34ad8c116abab6677344b74) 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: [RFC PATCH 0/3] arm64: Implement reliable stack trace
I can't see the original patch. Can the original poster (Mark B?) add me to Cc on the next version? It's also good practice to add lkml as well. That way, those of us not copied can at least find the patch in the archives. live-patch...@vger.kernel.org would also be a good idea for this one. On Thu, Oct 15, 2020 at 04:49:51PM +0100, Mark Brown wrote: > On Thu, Oct 15, 2020 at 03:16:12PM +0100, Mark Rutland wrote: > > On Thu, Oct 15, 2020 at 03:39:37PM +0200, Miroslav Benes wrote: > > > > I'll just copy an excerpt from my notes about the required guarantees. > > > Written by Josh (CCed, he has better idea about the problem than me > > > anyway). > > > > It also needs to: > > > - detect preemption / page fault frames and return an error > > > - only return success if it reaches the end of the task stack; for user > > > tasks, that means the syscall barrier; for kthreads/idle tasks, that > > > means finding a defined thread entry point > > > - make sure it can't get into a recursive loop > > > - make sure each return address is a valid text address > > > - properly detect generated code hacks like function graph tracing and > > > kretprobes > > > " > > > It would be great if we could put something like the above into the > > kernel tree, either under Documentation/ or in a comment somewhere for > > the reliable stacktrace functions. > > Yes, please - the expecations are quite hard to follow at the minute, > implementing it involves quite a bit of guesswork and cargo culting to > figure out what the APIs are supposed to do. Documentation is indeed long overdue. I suppose everyone's looking at me. I can do that, but my bandwidth's limited for at least a few weeks. [ Currently in week 4 of traveling cross-country with a camper ("caravan" in British-speak?), National Lampoon vacation style. ] If by cargo culting, you mean reverse engineering the requirements due to lack of documentation, that's fair. Otherwise, if you see anything that doesn't make sense or that can be improved, let me know. > > AFAICT, existing architectures don't always handle all of the above in > > arch_stack_walk_reliable(). For example, it looks like x86 assumes > > unwiding through exceptions is reliable for !CONFIG_FRAME_POINTER, but I > > think this might not always be true. Why not? What else are the existing arches missing from the above list? > I certainly wouldn't have inferred the list from what's there :/ Fair, presumably because of missing documentation. > The searching for a defined thread entry point for example isn't > entirely visible in the implementations. For now I'll speak only of x86, because I don't quite remember how powerpc does it. For thread entry points, aka the "end" of the stack: - For ORC, the end of the stack is either pt_regs, or -- when unwinding from kthreads, idle tasks, or irqs/exceptions in entry code -- UNWIND_HINT_EMPTY (found by the unwinder's check for orc->end. [ Admittedly the implementation needs to be cleaned up a bit. EMPTY is too broad and needs to be split into UNDEFINED and ENTRY. ] - For frame pointers, by convention, the end of the stack for all tasks is a defined stack offset: end of stack page - sizeof(pt_regs). And yes, all that needs to be documented. -- Josh
Re: [PATCH] irqchip: MST_IRQ should depend on ARCH_MEDIATEK or ARCH_MSTARV7
On Wed, 14 Oct 2020 15:17:03 +0200, Geert Uytterhoeven wrote: > The MStar interrupt controller is only found on MStar, SigmaStar, and > Mediatek SoCs. Hence add dependencies on ARCH_MEDIATEK and > ARCH_MSTARV7, to prevent asking the user about the MStar interrupt > controller driver when configuring a kernel without support for MStar, > SigmaStar, and Mediatek SoCs. Applied to irq/irqchip-next, thanks! [1/1] irqchip/mst: MST_IRQ should depend on ARCH_MEDIATEK or ARCH_MSTARV7 commit: 61b0648d569aca932eab87a67f7ca0ffd3ea2b68 Cheers, M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH v2 2/3] ARM: dts: rockchip: veyron: Remove 0 point from brightness-levels
On Wed, Oct 14, 2020 at 7:19 AM Daniel Thompson wrote: > > On Tue, Oct 13, 2020 at 01:01:02AM -0700, Alexandru Stan wrote: > > After the "PWM backlight interpolation adjustments" patches, the > > backlight interpolation works a little differently. The way these > > dts files were working before was relying on a bug (IMHO). > > > > Remove the 0-3 range since otherwise we would have a 252 long > > interpolation that would slowly go between 0 and 3, looking really bad > > in userspace. > > > > We don't need the 0% point, userspace seems to handle this just fine > > because it uses the bl_power property to turn off the display. > > > > Signed-off-by: Alexandru Stan > > Acked-by: Daniel Thompson Thank you! > > Note also shouldn't this be patch 1 of the set. AFAICT it makes sense > whether or not the interpolation algorithm is changed. Yeah, I guess it could be. Sorry I didn't think of it that way before, I'm used to landing things in a group. In particular on veyron I assume it will almost be a noop without having my driver patch (especially with the findings of 0% not being that important). Feel free to land this independently. > > > Daniel. Alexandru Stan (amstan)
Re: [RFC PATCH 0/3] Fix errors on DT overlay removal with devlinks
Hi Michael, On 10/14/20 2:36 PM, Michael Auchter wrote: > After updating to v5.9, I've started seeing errors in the kernel log > when using device tree overlays. Specifically, the problem seems to > happen when removing a device tree overlay that contains two devices > with some dependency between them (e.g., a device that provides a clock > and a device that consumes that clock). Removing such an overlay results > in: > > OF: ERROR: memory leak, expected refcount 1 instead of 2, > of_node_get()/of_node_put() unbalanced - destroy > OF: ERROR: memory leak, expected refcount 1 instead of 2, > of_node_get()/of_node_put() unbalanced - destroy > > followed by hitting some REFCOUNT_WARNs in refcount.c > > In the first patch, I've included a unittest that can be used to > reproduce this when built with CONFIG_OF_UNITTEST [1]. > > I believe the issue is caused by the cleanup performed when releasing > the devlink device that's created to represent the dependency between > devices. The devlink device has references to the consumer and supplier > devices, which it drops in device_link_free; the devlink device's > release callback calls device_link_free via call_srcu. > > When the overlay is being removed, all devices are removed, and > eventually the release callback for the devlink device run, and > schedules cleanup using call_srcu. Before device_link_free can and call > put_device on the consumer/supplier, the rest of the overlay removal > process runs, resulting in the error traces above. > > Patches 2 and 3 are an attempt at fixing this: call srcu_barrier to wait > for any pending device_link_free's to execute before continuing on with > the removal process. > > These patches resolve the issue, but probably not in the best way. In > particular, it seems strange to need to leak details of devlinks into > the device tree overlay code. So, I'd be curious to get some feedback or > hear any other ideas for how to resolve this issue. Thanks for finding the problem, analyzing it, creating a unittest, and creating a fix. I agree with your analysis that there are issues with the implementation of the test and fix. I'll dig into this to see if I can provide some useful improvements. -Frank > > Thanks, > Michael > > 1. Note that this isn't a very good unit test: it will report a "pass" >even if it fails with the aforementioned errors, as these errors >aren't propogated. > > Michael Auchter (3): > of: unittest: add test of overlay with devlinks > driver core: add device_links_barrier > of: dynamic: add device links barrier before detach > > drivers/base/core.c | 10 ++ > drivers/of/dynamic.c| 3 +++ > drivers/of/unittest-data/Makefile | 1 + > drivers/of/unittest-data/overlay_16.dts | 26 + > drivers/of/unittest.c | 16 +++ > include/linux/device.h | 1 + > 6 files changed, 57 insertions(+) > create mode 100644 drivers/of/unittest-data/overlay_16.dts >
Re: [PATCH v2] x86/insn, tools/x86: Fix some potential undefined behavior.
On October 15, 2020 9:12:16 AM PDT, Ian Rogers wrote: >From: Numfor Mbiziwo-Tiapo > >Don't perform unaligned loads in __get_next and __peek_nbyte_next as >these are forms of undefined behavior. > >These problems were identified using the undefined behavior sanitizer >(ubsan) with the tools version of the code and perf test. Part of this >patch was previously posted here: >https://lore.kernel.org/lkml/20190724184512.162887-4-n...@google.com/ > >v2. removes the validate_next check and merges the 2 changes into one >as >requested by Masami Hiramatsu > >Signed-off-by: Ian Rogers >Signed-off-by: Numfor Mbiziwo-Tiapo >--- > arch/x86/lib/insn.c | 4 ++-- > tools/arch/x86/lib/insn.c | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > >diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c >index 404279563891..be88ab250146 100644 >--- a/arch/x86/lib/insn.c >+++ b/arch/x86/lib/insn.c >@@ -20,10 +20,10 @@ > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) > > #define __get_next(t, insn) \ >- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; }) >+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += >sizeof(t); r; }) > > #define __peek_nbyte_next(t, insn, n) \ >- ({ t r = *(t*)((insn)->next_byte + n); r; }) >+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); r; }) > > #define get_next(t, insn) \ > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; >__get_next(t, insn); }) >diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c >index 0151dfc6da61..92358c71a59e 100644 >--- a/tools/arch/x86/lib/insn.c >+++ b/tools/arch/x86/lib/insn.c >@@ -20,10 +20,10 @@ > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) > > #define __get_next(t, insn) \ >- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; }) >+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += >sizeof(t); r; }) > > #define __peek_nbyte_next(t, insn, n) \ >- ({ t r = *(t*)((insn)->next_byte + n); r; }) >+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); r; }) > > #define get_next(t, insn) \ > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; >__get_next(t, insn); }) Wait, what? You are taking about x86-specific code, and on x86 unaligned memory accesses are supported, well-defined, and ubiquitous. This is B.S. at best, and unless the compiler turns the memcpy() right back into what you started with, deleterious for performance. If you have a *very* good reason for this kind of churn, wrap it in the unaligned access macros, but using memcpy() is insane. All you are doing is making the code worse. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: [PATCH 5/8] x86/clear_page: add clear_page_uncached()
On 2020-10-15 3:40 a.m., Borislav Petkov wrote: On Wed, Oct 14, 2020 at 08:21:57PM -0700, Ankur Arora wrote: Also, if we did extend clear_page() to take the page-size as parameter we still might not have enough information (ex. a 4K or a 2MB page that clear_page() sees could be part of a GUP of a much larger extent) to decide whether to go uncached or not. clear_page* assumes 4K. All of the lowlevel asm variants do. So adding the size there won't bring you a whole lot. So you'd need to devise this whole thing differently. Perhaps have a clear_pages() helper which decides based on size what to do: uncached clearing or the clear_page() as is now in a loop. I think that'll work well for GB pages, where the clear_pages() helper has enough information to make a decision. But, unless I'm missing something, I'm not sure how that would work for say, a 1GB mm_populate() using 4K pages. The clear_page() (or clear_pages()) in that case would only see the 4K size. But let me think about this more (and look at the callsites as you suggest.) Looking at the callsites would give you a better idea I'd say. Thanks, yeah that's a good idea. Let me go do that. Ankur Thx.
linux-next: manual merge of the wireless-drivers tree with the net tree
Hi all, Today's linux-next merge of the wireless-drivers tree got a conflict in: tools/testing/selftests/net/Makefile between commit: 1a01727676a8 ("selftests: Add VRF route leaking tests") from the net tree and commit: b7cc6d3c5c91 ("selftests: net: Add drop monitor test") from the wireless-drivers (presumably because it has merged part of the net-next tree) tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc tools/testing/selftests/net/Makefile index 3e7fb1e70c77,4773ce72edbd.. --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@@ -19,7 -19,7 +19,8 @@@ TEST_PROGS += txtimestamp.s TEST_PROGS += vrf-xfrm-tests.sh TEST_PROGS += rxtimestamp.sh TEST_PROGS += devlink_port_split.py + TEST_PROGS += drop_monitor_tests.sh +TEST_PROGS += vrf_route_leaking.sh TEST_PROGS_EXTENDED := in_netns.sh TEST_GEN_FILES = socket nettest TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any pgpFGPCzLbkLS.pgp Description: OpenPGP digital signature
[PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
There is a misconfiguration in the bios of the gpio pin used for the interrupt in the T490s. When interrupts are enabled in the tpm_tis driver code this results in an interrupt storm. This was initially reported when we attempted to enable the interrupt code in the tpm_tis driver, which previously wasn't setting a flag to enable it. Due to the reports of the interrupt storm that code was reverted and we went back to polling instead of using interrupts. Now that we know the T490s problem is a firmware issue, add code to check if the system is a T490s and disable interrupts if that is the case. This will allow us to enable interrupts for everyone else. If the user has a fixed bios they can force the enabling of interrupts with tpm_tis.interrupts=1 on the kernel command line. Cc: jar...@kernel.org Cc: Peter Huewe Cc: Jason Gunthorpe Cc: Hans de Goede Reviewed-by: James Bottomley Signed-off-by: Jerry Snitselaar --- drivers/char/tpm/tpm_tis.c | 29 +++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index 0b214963539d..4ed6e660273a 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -27,6 +27,7 @@ #include #include #include +#include #include "tpm.h" #include "tpm_tis_core.h" @@ -49,8 +50,8 @@ static inline struct tpm_tis_tcg_phy *to_tpm_tis_tcg_phy(struct tpm_tis_data *da return container_of(data, struct tpm_tis_tcg_phy, priv); } -static bool interrupts = true; -module_param(interrupts, bool, 0444); +static int interrupts = -1; +module_param(interrupts, int, 0444); MODULE_PARM_DESC(interrupts, "Enable interrupts"); static bool itpm; @@ -63,6 +64,28 @@ module_param(force, bool, 0444); MODULE_PARM_DESC(force, "Force device probe rather than using ACPI entry"); #endif +static int tpm_tis_disable_irq(const struct dmi_system_id *d) +{ + if (interrupts == -1) { + pr_notice("tpm_tis: %s detected: disabling interrupts.\n", d->ident); + interrupts = 0; + } + + return 0; +} + +static const struct dmi_system_id tpm_tis_dmi_table[] = { + { + .callback = tpm_tis_disable_irq, + .ident = "ThinkPad T490s", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T490s"), + }, + }, + {} +}; + #if defined(CONFIG_PNP) && defined(CONFIG_ACPI) static int has_hid(struct acpi_device *dev, const char *hid) { @@ -192,6 +215,8 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info) int irq = -1; int rc; + dmi_check_system(tpm_tis_dmi_table); + rc = check_acpi_tpm2(dev); if (rc) return rc; -- 2.28.0
[PATCH v2 1/4] block: keyslot-manager: Introduce passthrough keyslot manager
The device mapper may map over devices that have inline encryption capabilities, and to make use of those capabilities, the DM device must itself advertise those inline encryption capabilities. One way to do this would be to have the DM device set up a keyslot manager with a "sufficiently large" number of keyslots, but that would use a lot of memory. Also, the DM device itself has no "keyslots", and it doesn't make much sense to talk about "programming a key into a DM device's keyslot manager", so all that extra memory used to represent those keyslots is just wasted. All a DM device really needs to be able to do is advertise the crypto capabilities of the underlying devices in a coherent manner and expose a way to evict keys from the underlying devices. There are also devices with inline encryption hardware that do not have a limited number of keyslots. One can send a raw encryption key along with a bio to these devices (as opposed to typical inline encryption hardware that require users to first program a raw encryption key into a keyslot, and send the index of that keyslot along with the bio). These devices also only need the same things from the keyslot manager that DM devices need - a way to advertise crypto capabilities and potentially a way to expose a function to evict keys from hardware. So we introduce a "passthrough" keyslot manager that provides a way to represent a keyslot manager that doesn't have just a limited number of keyslots, and for which do not require keys to be programmed into keyslots. DM devices can set up a passthrough keyslot manager in their request queues, and advertise appropriate crypto capabilities based on those of the underlying devices. Blk-crypto does not attempt to program keys into any keyslots in the passthrough keyslot manager. Instead, if/when the bio is resubmitted to the underlying device, blk-crypto will try to program the key into the underlying device's keyslot manager. Signed-off-by: Satya Tangirala --- block/keyslot-manager.c | 41 + include/linux/keyslot-manager.h | 2 ++ 2 files changed, 43 insertions(+) diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c index 35abcb1ec051..5ad476dafeab 100644 --- a/block/keyslot-manager.c +++ b/block/keyslot-manager.c @@ -62,6 +62,11 @@ static inline void blk_ksm_hw_exit(struct blk_keyslot_manager *ksm) pm_runtime_put_sync(ksm->dev); } +static inline bool blk_ksm_is_passthrough(struct blk_keyslot_manager *ksm) +{ + return ksm->num_slots == 0; +} + /** * blk_ksm_init() - Initialize a keyslot manager * @ksm: The keyslot_manager to initialize. @@ -198,6 +203,10 @@ blk_status_t blk_ksm_get_slot_for_key(struct blk_keyslot_manager *ksm, int err; *slot_ptr = NULL; + + if (blk_ksm_is_passthrough(ksm)) + return BLK_STS_OK; + down_read(&ksm->lock); slot = blk_ksm_find_and_grab_keyslot(ksm, key); up_read(&ksm->lock); @@ -318,6 +327,16 @@ int blk_ksm_evict_key(struct blk_keyslot_manager *ksm, struct blk_ksm_keyslot *slot; int err = 0; + if (blk_ksm_is_passthrough(ksm)) { + if (ksm->ksm_ll_ops.keyslot_evict) { + blk_ksm_hw_enter(ksm); + err = ksm->ksm_ll_ops.keyslot_evict(ksm, key, -1); + blk_ksm_hw_exit(ksm); + return err; + } + return 0; + } + blk_ksm_hw_enter(ksm); slot = blk_ksm_find_keyslot(ksm, key); if (!slot) @@ -353,6 +372,9 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager *ksm) { unsigned int slot; + if (blk_ksm_is_passthrough(ksm)) + return; + /* This is for device initialization, so don't resume the device */ down_write(&ksm->lock); for (slot = 0; slot < ksm->num_slots; slot++) { @@ -394,3 +416,22 @@ void blk_ksm_unregister(struct request_queue *q) { q->ksm = NULL; } + +/** + * blk_ksm_init_passthrough() - Init a passthrough keyslot manager + * @ksm: The keyslot manager to init + * + * Initialize a passthrough keyslot manager. + * Called by e.g. storage drivers to set up a keyslot manager in their + * request_queue, when the storage driver wants to manage its keys by itself. + * This is useful for inline encryption hardware that doesn't have the concept + * of keyslots, and for layered devices. + * + * See blk_ksm_init() for more details about the parameters. + */ +void blk_ksm_init_passthrough(struct blk_keyslot_manager *ksm) +{ + memset(ksm, 0, sizeof(*ksm)); + init_rwsem(&ksm->lock); +} +EXPORT_SYMBOL_GPL(blk_ksm_init_passthrough); diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h index 18f3f5346843..323e15dd6fa7 100644 --- a/include/linux/keyslot-manager.h +++ b/include/linux/keyslot-manager.h @@ -103,4 +103,6 @@ void blk_ksm_reprogram_all_keys(struct blk_keyslot_manager
[PATCH v2 0/4] add support for inline encryption to device mapper
This patch series adds support for inline encryption to the device mapper. Patch 1 introduces the "passthrough" keyslot manager. The regular keyslot manager is designed for inline encryption hardware that have only a small fixed number of keyslots. A DM device itself does not actually have only a small fixed number of keyslots - it doesn't actually have any keyslots in the first place, and programming an encryption context into a DM device doesn't make much semantic sense. It is possible for a DM device to set up a keyslot manager with some "sufficiently large" number of keyslots in its request queue, so that upper layers can use the inline encryption capabilities of the DM device's underlying devices, but the memory being allocated for the DM device's keyslots is a waste since they won't actually be used by the DM device. The passthrough keyslot manager solves this issue - when the block layer sees that a request queue has a passthrough keyslot manager, it doesn't attempt to program any encryption context into the keyslot manager. The passthrough keyslot manager only allows the device to expose its inline encryption capabilities, and a way for upper layers to evict keys if necessary. There also exist inline encryption hardware that can handle encryption contexts directly, and allow users to pass them a data request along with the encryption context (as opposed to inline encryption hardware that require users to first program a keyslot with an encryption context, and then require the users to pass the keyslot index with the data request). Such devices can also make use of the passthrough keyslot manager. Patch 2 introduces a private field to struct blk_keyslot_manager that owners of the struct can use for any purpose. The struct blk_keyslot_manager has been embedded within other structures directly (like in struct ufs_hba in drivers/scsi/ufs/ufshcd.h), but we don't want to do that with struct mapped_device. So, the device mapper patches later in this series use the private field to hold a pointer to the associated struct mapped_device, since we can't use container_of() anymore. Patch 3 introduces the changes for inline encryption support for the device mapper. A DM device only exposes the intersection of the crypto capabilities of its underlying devices. This is so that in case a bio with an encryption context is eventually mapped to an underlying device that doesn't support that encryption context, the blk-crypto-fallback's cipher tfms are allocated ahead of time by the call to blk_crypto_start_using_key. Each DM target can now also specify that it "may_passthrough_inline_crypto" to opt-in to supporting passing through the underlying inline encryption capabilities. This flag is needed because it doesn't make much semantic sense for certain targets like dm-crypt to expose the underlying inline encryption capabilities to the upper layers. Again, the DM exposes inline encryption capabilities of the underlying devices only if all of them opt-in to passing through inline encryption support. A DM device's keyslot manager is set up whenever a new table is swapped in. This patch only allows the keyslot manager's capabilities to *expand* because of table changes. Any attempts to load a new table that would cause crypto capabilities to be dropped are rejected. The crypto capabilities of a new table are also verified when the table is loaded (and the load is rejected if crypto capabilities will be dropped because of the new table), but the keyslot manager for the DM device is only modified when the table is actually swapped in. This patch also only exposes the intersection of the underlying device's capabilities, which has the effect of causing en/decryption of a bio to fall back to the kernel crypto API (if the fallback is enabled) whenever any of the underlying devices doesn't support the encryption context of the bio - it might be possible to make the bio only fall back to the kernel crypto API if the bio's target underlying device doesn't support the bio's encryption context, but the use case may be uncommon enough in the first place not to warrant worrying about it right now. Patch 4 makes some DM targets opt-in to passing through inline encryption support. It does not (yet) try to enable this option with dm-raid, since users can "hot add" disks to a raid device, which makes this not completely straightforward (we'll need to ensure that any "hot added" disks must have a superset of the inline encryption capabilities of the rest of the disks in the raid device, due to the way Patch 2 of this series works). Changes v1 => v2: - Introduce private field to struct blk_keyslot_manager - Allow the DM keyslot manager to expand its crypto capabilities if the table is changed. - Make DM reject table changes that would otherwise cause crypto capabilities to be dropped. - Allocate the DM device's keyslot manager only when at least one crypto capability is supported (since a NULL value for q->ksm rep
[PATCH v2 3/4] dm: add support for passing through inline crypto support
Update the device-mapper core to support exposing the inline crypto support of the underlying device(s) through the device-mapper device. This works by creating a "passthrough keyslot manager" for the dm device, which declares support for encryption settings which all underlying devices support. When a supported setting is used, the bio cloning code handles cloning the crypto context to the bios for all the underlying devices. When an unsupported setting is used, the blk-crypto fallback is used as usual. Crypto support on each underlying device is ignored unless the corresponding dm target opts into exposing it. This is needed because for inline crypto to semantically operate on the original bio, the data must not be transformed by the dm target. Thus, targets like dm-linear can expose crypto support of the underlying device, but targets like dm-crypt can't. (dm-crypt could use inline crypto itself, though.) When a key is evicted from the dm device, it is evicted from all underlying devices. A DM device's table can only be changed if the "new" inline encryption capabilities are a superset of the "old" inline encryption capabilities. Attempts to make changes to the table that result in some inline encryption capability becoming no longer supported will be rejected. Co-developed-by: Eric Biggers Signed-off-by: Eric Biggers Signed-off-by: Satya Tangirala --- block/blk-crypto.c | 1 + block/keyslot-manager.c | 89 + drivers/md/dm-ioctl.c | 8 ++ drivers/md/dm.c | 217 +++- drivers/md/dm.h | 19 +++ include/linux/device-mapper.h | 6 + include/linux/keyslot-manager.h | 17 +++ 7 files changed, 356 insertions(+), 1 deletion(-) diff --git a/block/blk-crypto.c b/block/blk-crypto.c index 5da43f0973b4..c2be8f15006c 100644 --- a/block/blk-crypto.c +++ b/block/blk-crypto.c @@ -409,3 +409,4 @@ int blk_crypto_evict_key(struct request_queue *q, */ return blk_crypto_fallback_evict_key(key); } +EXPORT_SYMBOL_GPL(blk_crypto_evict_key); diff --git a/block/keyslot-manager.c b/block/keyslot-manager.c index 5ad476dafeab..e16e4a074765 100644 --- a/block/keyslot-manager.c +++ b/block/keyslot-manager.c @@ -416,6 +416,95 @@ void blk_ksm_unregister(struct request_queue *q) { q->ksm = NULL; } +EXPORT_SYMBOL_GPL(blk_ksm_unregister); + +/** + * blk_ksm_intersect_modes() - restrict supported modes by child device + * @parent: The keyslot manager for parent device + * @child: The keyslot manager for child device, or NULL + * + * Clear any crypto mode support bits in @parent that aren't set in @child. + * If @child is NULL, then all parent bits are cleared. + * + * Only use this when setting up the keyslot manager for a layered device, + * before it's been exposed yet. + */ +void blk_ksm_intersect_modes(struct blk_keyslot_manager *parent, +const struct blk_keyslot_manager *child) +{ + if (child) { + unsigned int i; + + parent->max_dun_bytes_supported = + min(parent->max_dun_bytes_supported, + child->max_dun_bytes_supported); + for (i = 0; i < ARRAY_SIZE(child->crypto_modes_supported); +i++) { + parent->crypto_modes_supported[i] &= + child->crypto_modes_supported[i]; + } + } else { + parent->max_dun_bytes_supported = 0; + memset(parent->crypto_modes_supported, 0, + sizeof(parent->crypto_modes_supported)); + } +} +EXPORT_SYMBOL_GPL(blk_ksm_intersect_modes); + +/** + * blk_ksm_is_superset() - Check if a KSM supports a superset of crypto modes + *and DUN bytes that another KSM supports. + * @ksm_superset: The KSM that we want to verify is a superset + * @ksm_subset: The KSM that we want to verify is a subset + * + * Return: True if @ksm_superset supports a superset of the crypto modes and DUN + *bytes that @ksm_subset supports. + */ +bool blk_ksm_is_superset(struct blk_keyslot_manager *ksm_superset, +struct blk_keyslot_manager *ksm_subset) +{ + int i; + + if (!ksm_subset) + return true; + + if (!ksm_superset) + return false; + + for (i = 0; i < ARRAY_SIZE(ksm_superset->crypto_modes_supported); i++) { + if (ksm_subset->crypto_modes_supported[i] & + (~ksm_superset->crypto_modes_supported[i])) { + return false; + } + } + + if (ksm_subset->max_dun_bytes_supported > + ksm_superset->max_dun_bytes_supported) { + return false; + } + + return true; +} +EXPORT_SYMBOL_GPL(blk_ksm_is_superset); + +/** + * blk_ksm_update_capabilities() - Update the restrictions of a KSM to those of + *
[PATCH v2 4/4] dm: enable may_passthrough_inline_crypto on some targets
dm-linear and dm-flakey obviously can pass through inline crypto support. Co-developed-by: Eric Biggers Signed-off-by: Eric Biggers Signed-off-by: Satya Tangirala --- drivers/md/dm-flakey.c | 1 + drivers/md/dm-linear.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c index a2cc9e45cbba..655286dacc35 100644 --- a/drivers/md/dm-flakey.c +++ b/drivers/md/dm-flakey.c @@ -253,6 +253,7 @@ static int flakey_ctr(struct dm_target *ti, unsigned int argc, char **argv) ti->num_discard_bios = 1; ti->per_io_data_size = sizeof(struct per_bio_data); ti->private = fc; + ti->may_passthrough_inline_crypto = true; return 0; bad: diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c index 00774b5d7668..345e22b9be5d 100644 --- a/drivers/md/dm-linear.c +++ b/drivers/md/dm-linear.c @@ -62,6 +62,7 @@ static int linear_ctr(struct dm_target *ti, unsigned int argc, char **argv) ti->num_secure_erase_bios = 1; ti->num_write_same_bios = 1; ti->num_write_zeroes_bios = 1; + ti->may_passthrough_inline_crypto = true; ti->private = lc; return 0; -- 2.29.0.rc1.297.gfa9743e501-goog
Re: [linux-stable-rc:linux-5.4.y 665/2391] drivers/android/binder.c:3776: Error: unrecognized keyword/register name `l.lwz
+openrisc folks On Thu, Oct 15, 2020 at 11:28 PM kernel test robot wrote: > tree: > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-5.4.y > head: 85b0841aab15c12948af951d477183ab3df7de14 > commit: c5665cafbedd2e2a523fe933e452391a02d3adb3 [665/2391] binder: Prevent > context manager from incrementing ref 0 > config: openrisc-randconfig-r002-20201014 (attached as .config) > compiler: or1k-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://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git/commit/?id=c5665cafbedd2e2a523fe933e452391a02d3adb3 > git remote add linux-stable-rc > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > git fetch --no-tags linux-stable-rc linux-5.4.y > git checkout c5665cafbedd2e2a523fe933e452391a02d3adb3 > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > ARCH=openrisc > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All errors (new ones prefixed by >>): > >drivers/android/binder.c: Assembler messages: > >> drivers/android/binder.c:3776: Error: unrecognized keyword/register name > >> `l.lwz ?ap,4(r25)' >drivers/android/binder.c:3781: Error: unrecognized keyword/register name > `l.addi ?ap,r0,0' binder is basically doing this: u64 data_ptr; if (get_user(data_ptr, (u64 __user *)ptr)) return -EFAULT; and GCC complains that that doesn't turn into valid assembly on openrisc, where get_user() of size 8 expands into this: #define __get_user_asm2(x, addr, err) \ { \ unsigned long long __gu_tmp;\ __asm__ __volatile__( \ "1: l.lwz %1,0(%2)\n" \ "2: l.lwz %H1,4(%2)\n" \ "3:\n" \ ".section .fixup,\"ax\"\n" \ "4: l.addi %0,r0,%3\n" \ " l.addi %1,r0,0\n" \ " l.addi %H1,r0,0\n" \ " l.j 3b\n" \ " l.nop\n"\ ".previous\n" \ ".section __ex_table,\"a\"\n" \ " .align 2\n" \ " .long 1b,4b\n" \ " .long 2b,4b\n" \ ".previous" \ : "=r"(err), "=&r"(__gu_tmp)\ : "r"(addr), "i"(-EFAULT), "0"(err)); \ (x) = (__typeof__(*(addr)))(\ (__typeof__((x)-(x)))__gu_tmp); \ } and apparently the "l.lwz %H1,4(%2)" and "l.addi %H1,r0,0" don't turn into valid assembly when %H1 expands to "?ap"? I don't know anything about OpenRISC, but this seems like it's probably an issue in the get_user() implementation.
[PATCH v2 2/4] block: add private field to struct keyslot_manager
Add a (void *) pointer to struct keyslot_manager that the owner of the struct can use for any purpose it wants. Right now, the struct keyslot_manager is expected to be embedded directly into other structs (and the owner of the keyslot_manager would use container_of() to access any other data the owner needs). However, this might take up more space than is acceptable, and it would be better to be able to add only a pointer to a struct keyslot_manager into other structs rather than embed the entire struct directly. But container_of() can't be used when only the pointer to the keyslot_manager is embded. The primary motivation of this patch is to get around that issue. Signed-off-by: Satya Tangirala --- include/linux/keyslot-manager.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/keyslot-manager.h b/include/linux/keyslot-manager.h index 323e15dd6fa7..37f1022b256f 100644 --- a/include/linux/keyslot-manager.h +++ b/include/linux/keyslot-manager.h @@ -59,6 +59,9 @@ struct blk_keyslot_manager { /* Device for runtime power management (NULL if none) */ struct device *dev; + /* Private data for owner */ + void *priv; + /* Here onwards are *private* fields for internal keyslot manager use */ unsigned int num_slots; -- 2.29.0.rc1.297.gfa9743e501-goog
Re: linux-next: manual merge of the usb tree with the pci tree
Hi all, On Mon, 21 Sep 2020 15:18:07 +1000 Stephen Rothwell wrote: > > Today's linux-next merge of the usb tree got a conflict in: > > drivers/pci/controller/pcie-brcmstb.c > > between commit: > > 1cf1b0a6dd95 ("PCI: brcmstb: Add bcm7278 register info") > > from the pci tree and commit: > > f48cc509c935 ("Revert "PCI: brcmstb: Wait for Raspberry Pi's firmware when > present"") > > from the usb tree. > > I fixed it up (see below) and can carry the fix as necessary. This > is now fixed as far as linux-next is concerned, but any non trivial > conflicts should be mentioned to your upstream maintainer when your tree > is submitted for merging. You may also want to consider cooperating > with the maintainer of the conflicting tree to minimise any particularly > complex conflicts. > > diff --cc drivers/pci/controller/pcie-brcmstb.c > index 6e7aa82a54a3,bac63d04297f.. > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@@ -1213,8 -929,6 +1211,7 @@@ static int brcm_pcie_probe(struct platf > { > struct device_node *np = pdev->dev.of_node, *msi_np; > struct pci_host_bridge *bridge; > - struct device_node *fw_np; > +const struct pcie_cfg_data *data; > struct brcm_pcie *pcie; > int ret; > This is now a conflict between the pci tree and Linus' tree. -- Cheers, Stephen Rothwell pgpTHoKAh_Xc4.pgp Description: OpenPGP digital signature
Re: [PATCH v2] x86/insn, tools/x86: Fix some potential undefined behavior.
On Thu, Oct 15, 2020 at 2:35 PM wrote: > > On October 15, 2020 9:12:16 AM PDT, Ian Rogers wrote: > >From: Numfor Mbiziwo-Tiapo > > > >Don't perform unaligned loads in __get_next and __peek_nbyte_next as > >these are forms of undefined behavior. > > > >These problems were identified using the undefined behavior sanitizer > >(ubsan) with the tools version of the code and perf test. Part of this > >patch was previously posted here: > >https://lore.kernel.org/lkml/20190724184512.162887-4-n...@google.com/ > > > >v2. removes the validate_next check and merges the 2 changes into one > >as > >requested by Masami Hiramatsu > > > >Signed-off-by: Ian Rogers > >Signed-off-by: Numfor Mbiziwo-Tiapo > >--- > > arch/x86/lib/insn.c | 4 ++-- > > tools/arch/x86/lib/insn.c | 4 ++-- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > >diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c > >index 404279563891..be88ab250146 100644 > >--- a/arch/x86/lib/insn.c > >+++ b/arch/x86/lib/insn.c > >@@ -20,10 +20,10 @@ > > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) > > > > #define __get_next(t, insn) \ > >- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; }) > >+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += > >sizeof(t); r; }) > > > > #define __peek_nbyte_next(t, insn, n) \ > >- ({ t r = *(t*)((insn)->next_byte + n); r; }) > >+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); r; }) > > > > #define get_next(t, insn) \ > > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; > >__get_next(t, insn); }) > >diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c > >index 0151dfc6da61..92358c71a59e 100644 > >--- a/tools/arch/x86/lib/insn.c > >+++ b/tools/arch/x86/lib/insn.c > >@@ -20,10 +20,10 @@ > > ((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr) > > > > #define __get_next(t, insn) \ > >- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; }) > >+ ({ t r; memcpy(&r, insn->next_byte, sizeof(t)); insn->next_byte += > >sizeof(t); r; }) > > > > #define __peek_nbyte_next(t, insn, n) \ > >- ({ t r = *(t*)((insn)->next_byte + n); r; }) > >+ ({ t r; memcpy(&r, (insn)->next_byte + n, sizeof(t)); r; }) > > > > #define get_next(t, insn) \ > > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; > >__get_next(t, insn); }) > > Wait, what? > > You are taking about x86-specific code, and on x86 unaligned memory accesses > are supported, well-defined, and ubiquitous. On why this is undefined behavior: https://lore.kernel.org/lkml/CAP-5=fU2XBoOa2=00vcuwyqsluzmsmzuxy63zjt9rz-nj+v...@mail.gmail.com/ > This is B.S. at best, and unless the compiler turns the memcpy() right back > into what you started with, deleterious for performance. On performance, the memcpys are fixed size and so lowered to loads on x86 by any reasonable compiler. See the thread above. > If you have a *very* good reason for this kind of churn, wrap it in the > unaligned access macros, but using memcpy() is insane. All you are doing is > making the code worse. The decoder is a shared code and using unaligned macros makes life hard for the other users of the code. Memcpy is the "standard" workaround for this kind of undefined behavior. https://lore.kernel.org/lkml/e4269cb2-d8e6-da26-6afd-a9df72d4b...@intel.com/ For motivation, beyond just having perf be sanitizer clean, see discussion here: https://lore.kernel.org/lkml/CAP-5=fuosgy3naztsbf3ylepabss7opsxlkcx36xkezzm34...@mail.gmail.com/ https://lore.kernel.org/lkml/160208761921.7002.1321765913567405137.tip-bot2@tip-bot2/ Thanks, Ian > -- > Sent from my Android device with K-9 Mail. Please excuse my brevity.
[tip:master] BUILD SUCCESS 3c095241232f26d9ca68df129cd6fa4e95bbfc31
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master branch HEAD: 3c095241232f26d9ca68df129cd6fa4e95bbfc31 Merge branch 'linus' elapsed time: 724m configs tested: 145 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig arm sama5_defconfig sparc sparc32_defconfig armmulti_v5_defconfig mipsgpr_defconfig arm pxa168_defconfig powerpc mpc834x_mds_defconfig arm cm_x300_defconfig arm zx_defconfig powerpc mpc8560_ads_defconfig powerpc tqm8555_defconfig arm alldefconfig mipsjmr3927_defconfig arm colibri_pxa300_defconfig armxcep_defconfig alpha defconfig powerpc eiger_defconfig mips ip32_defconfig powerpc motionpro_defconfig mips malta_defconfig arm lpc32xx_defconfig arm simpad_defconfig arm at91_dt_defconfig armlart_defconfig pariscgeneric-64bit_defconfig x86_64 alldefconfig riscv defconfig riscvnommu_k210_defconfig mips ip28_defconfig i386 alldefconfig arcvdk_hs38_defconfig powerpc pmac32_defconfig powerpc sbc8548_defconfig sh ul2_defconfig mips sb1250_swarm_defconfig armzeus_defconfig powerpc ep88xc_defconfig sh alldefconfig powerpc cm5200_defconfig arm collie_defconfig mips tb0287_defconfig powerpc mgcoge_defconfig arm tct_hammer_defconfig armvexpress_defconfig powerpc tqm8540_defconfig arm nhk8815_defconfig arm lubbock_defconfig sparc sparc64_defconfig arm pxa255-idp_defconfig s390 zfcpdump_defconfig sh se7705_defconfig powerpc mpc834x_itxgp_defconfig arm ep93xx_defconfig armspear3xx_defconfig powerpc mpc885_ads_defconfig sh polaris_defconfig powerpc powernv_defconfig shecovec24-romimage_defconfig powerpc ppc6xx_defconfig armmagician_defconfig shmigor_defconfig arm viper_defconfig sh sh7710voipgw_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a005-20201015 i386 randconfig-a006-20201015 i386 randconfig-a001-202
Re: [PATCH 2/3] dm: add support for passing through inline crypto support
On Thu, Sep 24, 2020 at 09:40:22AM -0400, Mike Snitzer wrote: > On Thu, Sep 24 2020 at 3:48am -0400, > Satya Tangirala wrote: > > > On Wed, Sep 23, 2020 at 09:21:03PM -0400, Mike Snitzer wrote: > > > On Wed, Sep 09 2020 at 7:44pm -0400, > > > Satya Tangirala wrote: > > > > > > > From: Eric Biggers > > > > > > > > Update the device-mapper core to support exposing the inline crypto > > > > support of the underlying device(s) through the device-mapper device. > > > > > > > > This works by creating a "passthrough keyslot manager" for the dm > > > > device, which declares support for encryption settings which all > > > > underlying devices support. When a supported setting is used, the bio > > > > cloning code handles cloning the crypto context to the bios for all the > > > > underlying devices. When an unsupported setting is used, the blk-crypto > > > > fallback is used as usual. > > > > > > > > Crypto support on each underlying device is ignored unless the > > > > corresponding dm target opts into exposing it. This is needed because > > > > for inline crypto to semantically operate on the original bio, the data > > > > must not be transformed by the dm target. Thus, targets like dm-linear > > > > can expose crypto support of the underlying device, but targets like > > > > dm-crypt can't. (dm-crypt could use inline crypto itself, though.) > > > > > > > > When a key is evicted from the dm device, it is evicted from all > > > > underlying devices. > > > > > > > > Signed-off-by: Eric Biggers > > > > Co-developed-by: Satya Tangirala > > > > Signed-off-by: Satya Tangirala > > > > --- > > > > block/blk-crypto.c | 1 + > > > > block/keyslot-manager.c | 34 > > > > drivers/md/dm-core.h| 4 ++ > > > > drivers/md/dm-table.c | 52 +++ > > > > drivers/md/dm.c | 92 - > > > > include/linux/device-mapper.h | 6 +++ > > > > include/linux/keyslot-manager.h | 7 +++ > > > > 7 files changed, 195 insertions(+), 1 deletion(-) > > > > > > > > > diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h > > > > index c4ef1fceead6..4542050eebfc 100644 > > > > --- a/drivers/md/dm-core.h > > > > +++ b/drivers/md/dm-core.h > > > > @@ -12,6 +12,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > > > > > #include > > > > > > > > @@ -49,6 +50,9 @@ struct mapped_device { > > > > > > > > int numa_node_id; > > > > struct request_queue *queue; > > > > +#ifdef CONFIG_BLK_INLINE_ENCRYPTION > > > > + struct blk_keyslot_manager ksm; > > > > +#endif > > > > > > > > atomic_t holders; > > > > atomic_t open_count; > > > > > > Any reason you placed the ksm member where you did? > > > > As in, any reason why it's placed right after the struct request_queue > > *queue? The ksm is going to be set up in the request_queue and is a part > > of the request_queue is some sense, so it seemed reasonable to me to > > group them togetherbut I don't think there's any reason it *has* to > > be there, if you think it should be put elsewhere (or maybe I'm > > misunderstanding your question :) ). > > Placing the full struct where you did is highly disruptive to the prior > care taken to tune alignment of struct members within mapped_device. > Ah I see - sorry about that! I ended up removing it entirely in the next version of this series while trying to address this and your other comments :). The next version is at https://lore.kernel.org/linux-block/20201015214632.41951-5-sat...@google.com/ > Switching to a pointer will be less so, but even still it might be best > to either find a hole in the struct (not looked recently, but there may > not be one) or simply put it at the end of the structure. > > The pahole utility is very useful for this kind of struct member > placement, etc. But it is increasingly unavailable in modern Linux > distros... > > Mike >
Re: [PATCH v2 2/3] selftests/run_kselftest.sh: Make each test individually selectable
On Thu, Oct 15, 2020 at 02:57:34PM +0530, Naresh Kamboju wrote: > On Tue, 29 Sep 2020 at 01:56, 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". > > > While testing this patch set on LAVA the skip test functionality is not > working. > We may have to revisit test definitions kselftest skip logic > or else > may add one more option to skip a given test on run_kselftest.sh script. > > ref: > https://github.com/Linaro/test-definitions/blob/master/automated/linux/kselftest/kselftest.sh#L196 Yes, LAVA's hack to skip tests needs to be adjusted. Here's what it should probably look like: https://github.com/Linaro/test-definitions/pull/231 -- Kees Cook
Re: [PATCH v2] net: usb: rtl8150: don't incorrectly assign random MAC addresses
Hi Greg, On Mon, 12 Oct 2020 09:14:28 +1100 Stephen Rothwell wrote: > > On Sun, 11 Oct 2020 23:00:30 +0530 Anant Thazhemadam > wrote: > > > > In set_ethernet_addr(), if get_registers() succeeds, the ethernet address > > that was read must be copied over. Otherwise, a random ethernet address > > must be assigned. > > > > get_registers() returns 0 if successful, and negative error number > > otherwise. However, in set_ethernet_addr(), this return value is > > incorrectly checked. > > > > Since this return value will never be equal to sizeof(node_id), a > > random MAC address will always be generated and assigned to the > > device; even in cases when get_registers() is successful. > > > > Correctly modifying the condition that checks if get_registers() was > > successful or not fixes this problem, and copies the ethernet address > > appropriately. > > > > Fixes: f45a4248ea4c ("net: usb: rtl8150: set random MAC address when > > set_ethernet_addr() fails") > > Signed-off-by: Anant Thazhemadam > > --- > > Changes in v2: > > * Fixed the format of the Fixes tag > > * Modified the commit message to better describe the issue being > > fixed > > > > +CCing Stephen and linux-next, since the commit fixed isn't in the > > networking > > tree, but is present in linux-next. > > > > drivers/net/usb/rtl8150.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c > > index f020401adf04..bf8a60533f3e 100644 > > --- a/drivers/net/usb/rtl8150.c > > +++ b/drivers/net/usb/rtl8150.c > > @@ -261,7 +261,7 @@ static void set_ethernet_addr(rtl8150_t *dev) > > > > ret = get_registers(dev, IDR, sizeof(node_id), node_id); > > > > - if (ret == sizeof(node_id)) { > > + if (!ret) { > > ether_addr_copy(dev->netdev->dev_addr, node_id); > > } else { > > eth_hw_addr_random(dev->netdev); > > -- > > 2.25.1 > > > > I will apply the above patch to the merge of the usb tree today to fix > up a semantic conflict between the usb tree and Linus' tree. It looks like you forgot to mention this one to Linus :-( It should probably say: Fixes: b2a0f274e3f7 ("net: rtl8150: Use the new usb control message API.") -- Cheers, Stephen Rothwell pgpI7gkNKtbKq.pgp Description: OpenPGP digital signature
Re: [PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
James should this get tacked on the end of your patchset? Regards, Jerry
Re: [PATCH] compiler.h: Clarify comment about the need for barrier_data()
On Thu, Oct 15, 2020 at 09:09:11PM +, David Laight wrote: > From: Arvind Sankar > > Sent: 15 October 2020 19:14 > > > > Be clear about @ptr vs the variable that @ptr points to, and add some > > more details as to why the special barrier_data() macro is required. > > > > Signed-off-by: Arvind Sankar > > --- > > include/linux/compiler.h | 33 ++--- > > 1 file changed, 22 insertions(+), 11 deletions(-) > > > > diff --git a/include/linux/compiler.h b/include/linux/compiler.h > > index 93035d7fee0d..d8cee7c8968d 100644 > > --- a/include/linux/compiler.h > > +++ b/include/linux/compiler.h > > @@ -86,17 +86,28 @@ void ftrace_likely_update(struct ftrace_likely_data *f, > > int val, > > > > #ifndef barrier_data > > /* > > - * This version is i.e. to prevent dead stores elimination on @ptr > > - * where gcc and llvm may behave differently when otherwise using > > - * normal barrier(): while gcc behavior gets along with a normal > > - * barrier(), llvm needs an explicit input variable to be assumed > > - * clobbered. The issue is as follows: while the inline asm might > > - * access any memory it wants, the compiler could have fit all of > > - * @ptr into memory registers instead, and since @ptr never escaped > > - * from that, it proved that the inline asm wasn't touching any of > > - * it. This version works well with both compilers, i.e. we're telling > > - * the compiler that the inline asm absolutely may see the contents > > - * of @ptr. See also: https://llvm.org/bugs/show_bug.cgi?id=15495 > > + * This version is to prevent dead stores elimination on @ptr where gcc and > > + * llvm may behave differently when otherwise using normal barrier(): > > while gcc > > + * behavior gets along with a normal barrier(), llvm needs an explicit > > input > > + * variable to be assumed clobbered. > > + * > > + * Its primary use is in implementing memzero_explicit(), which is used for > > + * clearing temporary data that may contain secrets. > > + * > > + * The issue is as follows: while the inline asm might access any memory it > > + * wants, the compiler could have fit all of the variable that @ptr points > > to > > + * into registers instead, and if @ptr never escaped from the function, it > > + * proved that the inline asm wasn't touching any of it. gcc only > > eliminates > > + * dead stores if the variable was actually allocated in registers, but > > llvm > > + * reasons that the variable _could_ have been in registers, so the inline > > asm > > + * can't reliably access it anyway, and eliminates dead stores even if the > > + * variable is actually in memory. > > I think I'd just say something like: > > Although the compiler must assume a "memory" clobber may affect all > memory, local variables (on stack) cannot actually be visible to the > asm unless their address has been passed to an external function. > So the compiler may assume such variables cannot be affected by > a normal asm volatile(::"memory") barrier(). > Passing the address of the local variables to the asm barrier > is enough to tell the compiler that the asm can 'see' the variables > (and spill anything held in registers to the stack) so that > the "memory" clobber has the expected effect. > > This is necessary to get llvm to do a memset() of on-stack data > at the end of a function to clear memory that contains secrets. > > David I think it's helpful to have the more detailed explanation about register variables -- at first glance, it's a bit mystifying as to why the compiler would think that the asm can't access the stack. Spilling registers to the stack is actually an undesirable side-effect of the workaround.
Re: [PATCH v3 0/3] Actually fix freelist pointer vs redzoning
On Thu, Oct 15, 2020 at 11:44:15AM +0200, Vlastimil Babka wrote: > On 10/15/20 10:23 AM, Christopher Lameter wrote: > > On Wed, 14 Oct 2020, Kees Cook wrote: > > > > > Note on patch 2: Christopher NAKed it, but I actually think this is a > > > reasonable thing to add -- the "too small" check is only made when built > > > with CONFIG_DEBUG_VM, so it *is* actually possible for someone to trip > > > over this directly, even if it would never make it into a released > > > kernel. I see no reason to just leave this foot-gun in place, though, so > > > we might as well just fix it too. (Which seems to be what Longman was > > > similarly supporting, IIUC.) > > > > Well then remove the duplication of checks. The NAK was there because it > > seems that you were not aware of the existing checks. > > > > > Anyway, if patch 2 stays NAKed, that's fine. It's entirely separable, > > > and the other 2 can land. :) > > > > Just deal with the old checks too and it will be fine. > > Yeah, the existing check is under CONFIG_DEBUG_VM, which means it's not > active on some configurations. Creating a cache is not exactly fast path > operation, so I would remove this guard. > As for the minimum size check, I would probably remove it (but watch out if > SLAB/SLOB can handle it). It's not effective to use slab cache for 4-byte > objects, but why make it an error. Err, why did the check exist to begin with? If the check isn't wanted, that's one thing, but I was just trying to fix what I saw in the redzone handling. What is preferred here? 1) drop patch 2 2) keep patch 2, but also: a) validate slab/slob can handle < word-sized allocations b) remove check in kmem_cache_sanity_check option 2a seems like it could be fragile if I miss something. I think I'd rather just take option 1. -- Kees Cook
Re: [PATCH 2/3] dm: add support for passing through inline crypto support
On Thu, Sep 24, 2020 at 10:23:54AM -0400, Mike Snitzer wrote: > On Thu, Sep 24 2020 at 3:38am -0400, > Satya Tangirala wrote: > > > On Wed, Sep 23, 2020 at 09:21:03PM -0400, Mike Snitzer wrote: > > > On Wed, Sep 09 2020 at 7:44pm -0400, > > > Satya Tangirala wrote: > > > > > > > From: Eric Biggers > > > > > > > > Update the device-mapper core to support exposing the inline crypto > > > > support of the underlying device(s) through the device-mapper device. > > > > > > > > This works by creating a "passthrough keyslot manager" for the dm > > > > device, which declares support for encryption settings which all > > > > underlying devices support. When a supported setting is used, the bio > > > > cloning code handles cloning the crypto context to the bios for all the > > > > underlying devices. When an unsupported setting is used, the blk-crypto > > > > fallback is used as usual. > > > > > > > > Crypto support on each underlying device is ignored unless the > > > > corresponding dm target opts into exposing it. This is needed because > > > > for inline crypto to semantically operate on the original bio, the data > > > > must not be transformed by the dm target. Thus, targets like dm-linear > > > > can expose crypto support of the underlying device, but targets like > > > > dm-crypt can't. (dm-crypt could use inline crypto itself, though.) > > > > > > > > When a key is evicted from the dm device, it is evicted from all > > > > underlying devices. > > > > > > > > Signed-off-by: Eric Biggers > > > > Co-developed-by: Satya Tangirala > > > > Signed-off-by: Satya Tangirala > > > > --- > > > > block/blk-crypto.c | 1 + > > > > block/keyslot-manager.c | 34 > > > > drivers/md/dm-core.h| 4 ++ > > > > drivers/md/dm-table.c | 52 +++ > > > > drivers/md/dm.c | 92 - > > > > include/linux/device-mapper.h | 6 +++ > > > > include/linux/keyslot-manager.h | 7 +++ > > > > 7 files changed, 195 insertions(+), 1 deletion(-) > > > > > > > > > diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h > > > > index c4ef1fceead6..4542050eebfc 100644 > > > > --- a/drivers/md/dm-core.h > > > > +++ b/drivers/md/dm-core.h > > > > @@ -12,6 +12,7 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > > > > > #include > > > > > > > > @@ -49,6 +50,9 @@ struct mapped_device { > > > > > > > > int numa_node_id; > > > > struct request_queue *queue; > > > > +#ifdef CONFIG_BLK_INLINE_ENCRYPTION > > > > + struct blk_keyslot_manager ksm; > > > > +#endif > > > > > > > > atomic_t holders; > > > > atomic_t open_count; > > > > > > Any reason you placed the ksm member where you did? > > > > > > Looking at 'struct blk_keyslot_manager' I'm really hating adding that > > > bloat to every DM device for a feature that really won't see much broad > > > use (AFAIK). > > > > > > Any chance you could allocate 'struct blk_keyslot_manager' as needed so > > > that most users of DM would only be carrying 1 extra pointer (set to > > > NULL)? > > > > I don't think there's any technical problem with doing that - the only > > other thing that would need addressing is that the patch uses > > "container_of" on that blk_keyslot_manager in dm_keyslot_evict() to get > > a pointer to the struct mapped_device. I could try adding a "private" > > field to struct blk_keyslot_manager and store a pointer to the struct > > mapped_device there). > > Yes, that'd be ideal. > > As for the lifetime of the struct blk_keyslot_manager pointer DM would > manage (in your future code revision): you meantioned in one reply that > the request_queue takes care of setting up the ksm... but the ksm > is tied to the queue at a later phase using blk_ksm_register(). > I probably wasn't clear in that reply :(. So the request_queue isn't responsible for setting up the ksm - setting up the ksm in the request queue is the responsibility of the DM device. > In any case, I think my feature reequest (to have DM allocate the ksm > struct only as needed) is a bit challenging because of how DM allocates > the request_queue upfront in alloc_dev() and then later completes the > request_queue initialization based on DM_TYPE* in dm_setup_md_queue(). > > It _could_ be that you'll need to add a new DM_TYPE_KSM_BIO_BASED or > something. But you have a catch-22 in that the dm-table.c code to > establish the intersection of supported modes assumes ksm is already > allocated. So something needs to give by reasoning through: _what_ is > the invariant that will trigger the delayed allocation of the ksm > struct? I don't yet see how you can make that informed decision that > the target(s) in the DM table _will_ use the ksm if it exists. > What I tried doing in the next version that I just sent out was to get the DM device to set up the ksm as appropriate on table swaps (and also to verify
Re: [GIT PULL] Kunit fixes update for Linux 5.10-rc1
The pull request you sent on Thu, 15 Oct 2020 15:13:02 -0600: > git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest > tags/linux-kselftest-kunit-fixes-5.10-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/578a7155c5a1894a789d4ece181abf9d25dc6b0d Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] HID for 5.10
The pull request you sent on Thu, 15 Oct 2020 20:52:36 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/bf36c6b946c8895cf590f10dbd70b589b0dc101f Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] trivial for 5.10
The pull request you sent on Thu, 15 Oct 2020 20:56:41 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/jikos/trivial.git for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/bbf625990371782370f6eacb3155dc1fe131ddfc Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] livepatching for 5.10
The pull request you sent on Thu, 15 Oct 2020 20:31:55 +0200 (CEST): > git://git.kernel.org/pub/scm/linux/kernel/git/livepatching/livepatching > for-linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/0cd7d9795fa82226e7516d38b474bddae8b1ff26 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] configfs updates for 5.10
The pull request you sent on Thu, 15 Oct 2020 19:54:01 +0200: > git://git.infradead.org/users/hch/configfs.git tags/configfs-5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/ca5387e448e1f88440dc93e143b353592f8a8af6 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH 1/2] arm64: dts: qcom: sc7180: Add gpu cooling support
Hi, On Thu, Oct 15, 2020 at 12:07:01AM +0530, man...@codeaurora.org wrote: > On 2020-10-14 18:59, Akhil P Oommen wrote: > > On 10/9/2020 10:27 PM, Matthias Kaehlcke wrote: > > > On Fri, Oct 09, 2020 at 08:05:10AM -0700, Doug Anderson wrote: > > > > Hi, > > > > > > > > On Thu, Oct 8, 2020 at 10:10 AM Akhil P Oommen > > > > wrote: > > > > > > > > > > Add cooling-cells property and the cooling maps for the gpu tzones > > > > > to support GPU cooling. > > > > > > > > > > Signed-off-by: Akhil P Oommen > > > > > --- > > > > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 29 > > > > > ++--- > > > > > 1 file changed, 22 insertions(+), 7 deletions(-) > > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > > b/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > > index d46b383..40d6a28 100644 > > > > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > > @@ -2,7 +2,7 @@ > > > > > /* > > > > >* SC7180 SoC device tree source > > > > >* > > > > > - * Copyright (c) 2019, The Linux Foundation. All rights reserved. > > > > > + * Copyright (c) 2019-20, The Linux Foundation. All rights > > > > > reserved. > > > > >*/ > > > > > > > > > > #include > > > > > @@ -1885,6 +1885,7 @@ > > > > > iommus = <&adreno_smmu 0>; > > > > > operating-points-v2 = <&gpu_opp_table>; > > > > > qcom,gmu = <&gmu>; > > > > > + #cooling-cells = <2>; > > > > > > > > Presumably we should add this to the devicetree bindings, too? > > Yes, thanks for catching this. Will update in the next patch. > > > > > > > > > > > > > > > interconnects = <&gem_noc > > > > > MASTER_GFX3D &mc_virt SLAVE_EBI1>; > > > > > interconnect-names = "gfx-mem"; > > > > > @@ -3825,16 +3826,16 @@ > > > > > }; > > > > > > > > > > gpuss0-thermal { > > > > > - polling-delay-passive = <0>; > > > > > + polling-delay-passive = <100>; > > > > > > > > Why did you make this change? I'm pretty sure that we _don't_ want > > > > this since we're using interrupts for the thermal sensor. See commit > > > > 22337b91022d ("arm64: dts: qcom: sc7180: Changed polling mode in > > > > Thermal-zones node"). > > > > > > I was going to ask the same, this shouldn't be needed. > As per our understanding unlike "polling-delay", this delay property is > intended to activate polling thread on post trip threshold violation and it > is irrespective of sensor is capable for trip interrupt or not. > This polling is more of governor related. Below are the few references from > Documentation/code which tells polling-delay-passive is needed for IPA for > better IPA performance. > > As per Power allocator documentations > > 1. > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/driver-api/thermal/power_allocator.rst?h=v5.4.71#n264 > > "The power allocator governor's PID controller works best if there is a > periodic tick. If you have a driver that calls > `thermal_zone_device_update()` (or anything that ends up calling the > governor's `throttle()` function) repetitively, the governor response > won't be very good. Note that this is not particular to this > governor, step-wise will also misbehave if you call its throttle() > faster than the normal thermal framework tick (due to interrupts for > example) as it will overreact" > > 2. In Power allocator code, when switch_on/control trip temp violation, it > is enabling passive counter to activate passive polling @ > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/thermal/power_allocator.c?h=v5.4.71#n634 > > 3. while calculating derivative term, it is using passive_delay @ > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/thermal/power_allocator.c?h=v5.4.71#n243 > > 4. Sensor interrupt will work if temperature is fluctuating between > trip_temp and hysteresis. But say a case where we are not enabling > polling-delay-passive. In this case if current temperature > control_temp > trip(2nd passive trip) and > temperature trend is still raising, then sensor high trip will be disabled > (OR configured for critical trip threshold). No more trip interrupt from > sensor until it reaches critical trip or falls below control_temp > hysteresis. > How the governor re-evaluate its next mitigation without passive polling > thread here ? > > I think the same is required for CPU thermal zone as well. Thanks for the explication and pointers! I ran some tests to re-confirm. For that I lowered the trip point temperatures of CPU6 to 60/70, to make it easier to trigger throttling without necessarily affecting the other CPUs. Further I enabled tracing for the events 'thermal_temperature', 'thermal_zone_trip' and 'thermal_power_allocator'.
Re: [GIT PULL] dma-mapping updates for 5.10
The pull request you sent on Thu, 15 Oct 2020 19:47:43 +0200: > git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/5a32c3413d3340f90c82c84b375ad4b335a59f28 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] dmaengine updates for v5.10-rc1
The pull request you sent on Thu, 15 Oct 2020 12:36:22 +0530: > git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git > tags/dmaengine-5.10-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/f065199d4df0b1512f935621d2de128ddb3fcc3a Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] Kselftest next update for Linux 5.10-rc1
The pull request you sent on Thu, 15 Oct 2020 13:55:07 -0600: > git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest > tags/linux-kselftest-next-5.10-rc1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/0674324b16d40e14b9d8ea2d667627c010608c28 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH v2] net: usb: rtl8150: don't incorrectly assign random MAC addresses
On Fri, 16 Oct 2020 08:59:22 +1100 Stephen Rothwell wrote: > > I will apply the above patch to the merge of the usb tree today to fix > > up a semantic conflict between the usb tree and Linus' tree. > > It looks like you forgot to mention this one to Linus :-( > > It should probably say: > > Fixes: b2a0f274e3f7 ("net: rtl8150: Use the new usb control message API.") Umpf. I think Greg sent his changes first, so this is on me. The networking PR is still outstanding, can we reply to the PR with the merge guidance. Or is it too late?
[PATCH 2/2] tools/include: Add rtnetlink.h, id_addr.h and neighbour.h
tools/lib/bpf/netlink.c depends on rtnetlink.h and netlink.h (via nlattr.h). Older versions of rtnetlink.h and netlink.h can cause duplicate conflicting definitions to occur, as things like header guards don't agree. To avoid these mismatches add rtnetlink.h, if_addr.h and neighbour.h to tools/include with the standard difference check. Signed-off-by: Ian Rogers --- tools/include/uapi/linux/if_addr.h | 72 +++ tools/include/uapi/linux/neighbour.h | 199 +++ tools/include/uapi/linux/rtnetlink.h | 787 +++ tools/lib/bpf/Makefile | 9 + 4 files changed, 1067 insertions(+) create mode 100644 tools/include/uapi/linux/if_addr.h create mode 100644 tools/include/uapi/linux/neighbour.h create mode 100644 tools/include/uapi/linux/rtnetlink.h diff --git a/tools/include/uapi/linux/if_addr.h b/tools/include/uapi/linux/if_addr.h new file mode 100644 index ..dfcf3ce0097f --- /dev/null +++ b/tools/include/uapi/linux/if_addr.h @@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef __LINUX_IF_ADDR_H +#define __LINUX_IF_ADDR_H + +#include +#include + +struct ifaddrmsg { + __u8ifa_family; + __u8ifa_prefixlen; /* The prefix length*/ + __u8ifa_flags; /* Flags*/ + __u8ifa_scope; /* Address scope*/ + __u32 ifa_index; /* Link index */ +}; + +/* + * Important comment: + * IFA_ADDRESS is prefix address, rather than local interface address. + * It makes no difference for normally configured broadcast interfaces, + * but for point-to-point IFA_ADDRESS is DESTINATION address, + * local address is supplied in IFA_LOCAL attribute. + * + * IFA_FLAGS is a u32 attribute that extends the u8 field ifa_flags. + * If present, the value from struct ifaddrmsg will be ignored. + */ +enum { + IFA_UNSPEC, + IFA_ADDRESS, + IFA_LOCAL, + IFA_LABEL, + IFA_BROADCAST, + IFA_ANYCAST, + IFA_CACHEINFO, + IFA_MULTICAST, + IFA_FLAGS, + IFA_RT_PRIORITY, /* u32, priority/metric for prefix route */ + IFA_TARGET_NETNSID, + __IFA_MAX, +}; + +#define IFA_MAX (__IFA_MAX - 1) + +/* ifa_flags */ +#define IFA_F_SECONDARY0x01 +#define IFA_F_TEMPORARYIFA_F_SECONDARY + +#defineIFA_F_NODAD 0x02 +#define IFA_F_OPTIMISTIC 0x04 +#define IFA_F_DADFAILED0x08 +#defineIFA_F_HOMEADDRESS 0x10 +#define IFA_F_DEPRECATED 0x20 +#define IFA_F_TENTATIVE0x40 +#define IFA_F_PERMANENT0x80 +#define IFA_F_MANAGETEMPADDR 0x100 +#define IFA_F_NOPREFIXROUTE0x200 +#define IFA_F_MCAUTOJOIN 0x400 +#define IFA_F_STABLE_PRIVACY 0x800 + +struct ifa_cacheinfo { + __u32 ifa_prefered; + __u32 ifa_valid; + __u32 cstamp; /* created timestamp, hundredths of seconds */ + __u32 tstamp; /* updated timestamp, hundredths of seconds */ +}; + +/* backwards compatibility for userspace */ +#ifndef __KERNEL__ +#define IFA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ifaddrmsg +#define IFA_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct ifaddrmsg)) +#endif + +#endif diff --git a/tools/include/uapi/linux/neighbour.h b/tools/include/uapi/linux/neighbour.h new file mode 100644 index ..dc8b72201f6c --- /dev/null +++ b/tools/include/uapi/linux/neighbour.h @@ -0,0 +1,199 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ +#ifndef __LINUX_NEIGHBOUR_H +#define __LINUX_NEIGHBOUR_H + +#include +#include + +struct ndmsg { + __u8ndm_family; + __u8ndm_pad1; + __u16 ndm_pad2; + __s32 ndm_ifindex; + __u16 ndm_state; + __u8ndm_flags; + __u8ndm_type; +}; + +enum { + NDA_UNSPEC, + NDA_DST, + NDA_LLADDR, + NDA_CACHEINFO, + NDA_PROBES, + NDA_VLAN, + NDA_PORT, + NDA_VNI, + NDA_IFINDEX, + NDA_MASTER, + NDA_LINK_NETNSID, + NDA_SRC_VNI, + NDA_PROTOCOL, /* Originator of entry */ + NDA_NH_ID, + NDA_FDB_EXT_ATTRS, + __NDA_MAX +}; + +#define NDA_MAX (__NDA_MAX - 1) + +/* + * Neighbor Cache Entry Flags + */ + +#define NTF_USE0x01 +#define NTF_SELF 0x02 +#define NTF_MASTER 0x04 +#define NTF_PROXY 0x08/* == ATF_PUBL */ +#define NTF_EXT_LEARNED0x10 +#define NTF_OFFLOADED 0x20 +#define NTF_STICKY 0x40 +#define NTF_ROUTER 0x80 + +/* + * Neighbor Cache Entry States. + */ + +#define NUD_INCOMPLETE 0x01 +#define NUD_REACHABLE 0x02 +#define NUD_STALE 0x04 +#define NUD_DELAY 0x08 +#define NUD_PROBE 0x10 +#define NUD_FAILED 0x20 + +/* Dummy states */ +#define NUD_NOARP 0x40 +#define NUD_PERMANENT
Re: [PATCH v8 3/3] leds: trigger: implement a tty trigger
Hi "Uwe, I love your patch! Perhaps something to improve: [auto build test WARNING on tty/tty-testing] [also build test WARNING on pavel-linux-leds/for-next linus/master j.anaszewski-leds/for-next v5.9 next-20201015] [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/Uwe-Kleine-K-nig/leds-trigger-implement-a-tty-trigger/20201012-203510 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing config: openrisc-randconfig-s031-20201015 (attached as .config) compiler: or1k-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-rc1-dirty # https://github.com/0day-ci/linux/commit/4185c273a8de0340cee6655a363f98c3737665d0 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Uwe-Kleine-K-nig/leds-trigger-implement-a-tty-trigger/20201012-203510 git checkout 4185c273a8de0340cee6655a363f98c3737665d0 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot "sparse warnings: (new ones prefixed by >>)" >> drivers/leds/trigger/ledtrig-tty.c:177:20: sparse: sparse: symbol >> 'ledtrig_tty' was not declared. Should it be static? Please review and possibly fold the followup patch. --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
[PATCH 1/2] tools/include: Update if_link.h and netlink.h
These are tested to be the latest as part of the tools/lib/bpf build. Signed-off-by: Ian Rogers --- tools/include/uapi/linux/if_link.h | 269 + tools/include/uapi/linux/netlink.h | 107 2 files changed, 342 insertions(+), 34 deletions(-) diff --git a/tools/include/uapi/linux/if_link.h b/tools/include/uapi/linux/if_link.h index 781e482dc499..c4b23f06f69e 100644 --- a/tools/include/uapi/linux/if_link.h +++ b/tools/include/uapi/linux/if_link.h @@ -7,24 +7,23 @@ /* This struct should be in sync with struct rtnl_link_stats64 */ struct rtnl_link_stats { - __u32 rx_packets; /* total packets received */ - __u32 tx_packets; /* total packets transmitted*/ - __u32 rx_bytes; /* total bytes received */ - __u32 tx_bytes; /* total bytes transmitted */ - __u32 rx_errors; /* bad packets received */ - __u32 tx_errors; /* packet transmit problems */ - __u32 rx_dropped; /* no space in linux buffers*/ - __u32 tx_dropped; /* no space available in linux */ - __u32 multicast; /* multicast packets received */ + __u32 rx_packets; + __u32 tx_packets; + __u32 rx_bytes; + __u32 tx_bytes; + __u32 rx_errors; + __u32 tx_errors; + __u32 rx_dropped; + __u32 tx_dropped; + __u32 multicast; __u32 collisions; - /* detailed rx_errors: */ __u32 rx_length_errors; - __u32 rx_over_errors; /* receiver ring buff overflow */ - __u32 rx_crc_errors; /* recved pkt with crc error*/ - __u32 rx_frame_errors;/* recv'd frame alignment error */ - __u32 rx_fifo_errors; /* recv'r fifo overrun */ - __u32 rx_missed_errors; /* receiver missed packet */ + __u32 rx_over_errors; + __u32 rx_crc_errors; + __u32 rx_frame_errors; + __u32 rx_fifo_errors; + __u32 rx_missed_errors; /* detailed tx_errors */ __u32 tx_aborted_errors; @@ -37,29 +36,200 @@ struct rtnl_link_stats { __u32 rx_compressed; __u32 tx_compressed; - __u32 rx_nohandler; /* dropped, no handler found*/ + __u32 rx_nohandler; }; -/* The main device statistics structure */ +/** + * struct rtnl_link_stats64 - The main device statistics structure. + * + * @rx_packets: Number of good packets received by the interface. + * For hardware interfaces counts all good packets received from the device + * by the host, including packets which host had to drop at various stages + * of processing (even in the driver). + * + * @tx_packets: Number of packets successfully transmitted. + * For hardware interfaces counts packets which host was able to successfully + * hand over to the device, which does not necessarily mean that packets + * had been successfully transmitted out of the device, only that device + * acknowledged it copied them out of host memory. + * + * @rx_bytes: Number of good received bytes, corresponding to @rx_packets. + * + * For IEEE 802.3 devices should count the length of Ethernet Frames + * excluding the FCS. + * + * @tx_bytes: Number of good transmitted bytes, corresponding to @tx_packets. + * + * For IEEE 802.3 devices should count the length of Ethernet Frames + * excluding the FCS. + * + * @rx_errors: Total number of bad packets received on this network device. + * This counter must include events counted by @rx_length_errors, + * @rx_crc_errors, @rx_frame_errors and other errors not otherwise + * counted. + * + * @tx_errors: Total number of transmit problems. + * This counter must include events counter by @tx_aborted_errors, + * @tx_carrier_errors, @tx_fifo_errors, @tx_heartbeat_errors, + * @tx_window_errors and other errors not otherwise counted. + * + * @rx_dropped: Number of packets received but not processed, + * e.g. due to lack of resources or unsupported protocol. + * For hardware interfaces this counter should not include packets + * dropped by the device which are counted separately in + * @rx_missed_errors (since procfs folds those two counters together). + * + * @tx_dropped: Number of packets dropped on their way to transmission, + * e.g. due to lack of resources. + * + * @multicast: Multicast packets received. + * For hardware interfaces this statistic is commonly calculated + * at the device level (unlike @rx_packets) and therefore may include + * packets which did not reach the host. + * + * For IEEE 802.3 devices this counter may be equivalent to: + * + *- 30.3.1.1.21 aMulticastFramesReceivedOK + * + * @collisions: Number of collisions during packet transmissions. + * + * @rx_length_errors: Number of packets dropped due to invalid length. + * Par
[RFC PATCH] leds: trigger: ledtrig_tty can be static
Signed-off-by: kernel test robot --- ledtrig-tty.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/leds/trigger/ledtrig-tty.c b/drivers/leds/trigger/ledtrig-tty.c index 806548e33cd874..09cba818fb65c7 100644 --- a/drivers/leds/trigger/ledtrig-tty.c +++ b/drivers/leds/trigger/ledtrig-tty.c @@ -174,7 +174,7 @@ static void ledtrig_tty_deactivate(struct led_classdev *led_cdev) kfree(trigger_data); } -struct led_trigger ledtrig_tty = { +static struct led_trigger ledtrig_tty = { .name = "tty", .activate = ledtrig_tty_activate, .deactivate = ledtrig_tty_deactivate,
Re: [PATCH] [PATCH] [v3] wireless: Initial driver submission for pureLiFi STA devices
On Wed, 2020-10-14 at 11:49 +0530, Srinivasan Raju wrote: > This introduces the pureLiFi LiFi driver for LiFi-X, LiFi-XC > and LiFi-XL USB devices. trivia: netdev_ might be better than dev_. > diff --git a/drivers/net/wireless/purelifi/chip.c > b/drivers/net/wireless/purelifi/chip.c [] > +int purelifi_chip_switch_radio(struct purelifi_chip *chip, u32 value) > +{ > + int r; > + > + r = usb_write_req((const u8 *)&value, sizeof(value), USB_REQ_POWER_WR); > + if (r) > + dev_err(purelifi_chip_dev(chip), "%s r=%d", __func__, r); missing '\n' termination. [] > > +int purelifi_chip_set_rate(struct purelifi_chip *chip, u8 rate) > +{ > + int r; > + static struct purelifi_chip *chip_p; > + > + if (chip) > + chip_p = chip; > + if (!chip_p) > + return -EINVAL; > + > + r = usb_write_req((const u8 *)&rate, sizeof(rate), USB_REQ_RATE_WR); > + if (r) > + dev_err(purelifi_chip_dev(chip), "set rate failed r=%d", r); same missing newline, etc It might also be better to use a consistent error message like dev_err(purelifi_chip_dev(chip), "%s: failed, r=%d\n", r); for both. > diff --git a/drivers/net/wireless/purelifi/mac.c > b/drivers/net/wireless/purelifi/mac.c [] > +static const struct ieee80211_rate purelifi_rates[] = { > + { .bitrate = 10, > + .hw_value = PURELIFI_CCK_RATE_1M, }, [] > + { .bitrate = 60, > + .hw_value = PURELIFI_OFDM_RATE_6M, > + .flags = 0 }, Seems odd to set .flags to 0 for only some of the entries. Perhaps more readable to always leave it off if unset. > + { .bitrate = 90, > + .hw_value = PURELIFI_OFDM_RATE_9M, > + .flags = 0 }, > + { .bitrate = 120, > + .hw_value = PURELIFI_OFDM_RATE_12M, > + .flags = 0 }, > + { .bitrate = 180, > + .hw_value = PURELIFI_OFDM_RATE_18M, > + .flags = 0 }, > + { .bitrate = 240, > + .hw_value = PURELIFI_OFDM_RATE_24M, > + .flags = 0 }, > + { .bitrate = 360, > + .hw_value = PURELIFI_OFDM_RATE_36M, > + .flags = 0 }, > + { .bitrate = 480, > + .hw_value = PURELIFI_OFDM_RATE_48M, > + .flags = 0 }, > + { .bitrate = 540, > + .hw_value = PURELIFI_OFDM_RATE_54M, > + .flags = 0 }, > +}; [] > +int purelifi_mac_init_hw(struct ieee80211_hw *hw) > +{ > + int r; > + struct purelifi_mac *mac = purelifi_hw_mac(hw); > + struct purelifi_chip *chip = &mac->chip; > + > + r = purelifi_chip_init_hw(chip); > + if (r) { > + dev_warn(purelifi_mac_dev(mac), "init hw failed. (%d)\n", r); > + goto out; > + } > + > + dev_dbg(purelifi_mac_dev(mac), "irq_disabled %d", irqs_disabled()); missing newline > +static int filter_ack(struct ieee80211_hw *hw, struct ieee80211_hdr *rx_hdr, > + struct ieee80211_rx_status *stats) > +{ > + struct purelifi_mac *mac = purelifi_hw_mac(hw); > + struct sk_buff *skb; > + struct sk_buff_head *q; > + unsigned long flags; > + int found = 0; > + int i, position = 0; > + > + if (!ieee80211_is_ack(rx_hdr->frame_control)) > + return 0; > + dev_info(purelifi_mac_dev(mac), "%s::ACK Received", __func__); missing newline > +static int purelifi_op_add_interface(struct ieee80211_hw *hw, > + struct ieee80211_vif *vif) > +{ > + struct purelifi_mac *mac = purelifi_hw_mac(hw); > + static const char * const iftype80211[] = { > + [NL80211_IFTYPE_STATION]= "Station", > + [NL80211_IFTYPE_ADHOC] = "Adhoc" > + }; > + > + if (mac->type != NL80211_IFTYPE_UNSPECIFIED) > + return -EOPNOTSUPP; > + > + if (vif->type == NL80211_IFTYPE_ADHOC || > + vif->type == NL80211_IFTYPE_STATION) { > + dev_info(purelifi_mac_dev(mac), "%s %s\n", __func__, > + iftype80211[vif->type]); > + mac->type = vif->type; > + mac->vif = vif; > + } else { > + dev_info(purelifi_mac_dev(mac), "unsupported iftype"); etc...
Re: [PATCH v2] net: usb: rtl8150: don't incorrectly assign random MAC addresses
On Thu, 15 Oct 2020 15:24:51 -0700 Jakub Kicinski wrote: > On Fri, 16 Oct 2020 08:59:22 +1100 Stephen Rothwell wrote: > > > I will apply the above patch to the merge of the usb tree today to fix > > > up a semantic conflict between the usb tree and Linus' tree. > > > > It looks like you forgot to mention this one to Linus :-( > > > > It should probably say: > > > > Fixes: b2a0f274e3f7 ("net: rtl8150: Use the new usb control message API.") > > Umpf. I think Greg sent his changes first, so this is on me. > > The networking PR is still outstanding, can we reply to the PR with > the merge guidance. Or is it too late? I take that back, this came through net, not net-next so our current PR is irrelevant :)
Re: [PATCH] tpm_tis: Disable interrupts on ThinkPad T490s
On Thu, Oct 15, 2020 at 2:44 PM Jerry Snitselaar wrote: > > There is a misconfiguration in the bios of the gpio pin used for the > interrupt in the T490s. When interrupts are enabled in the tpm_tis > driver code this results in an interrupt storm. This was initially > reported when we attempted to enable the interrupt code in the tpm_tis > driver, which previously wasn't setting a flag to enable it. Due to > the reports of the interrupt storm that code was reverted and we went back > to polling instead of using interrupts. Now that we know the T490s problem > is a firmware issue, add code to check if the system is a T490s and > disable interrupts if that is the case. This will allow us to enable > interrupts for everyone else. If the user has a fixed bios they can > force the enabling of interrupts with tpm_tis.interrupts=1 on the > kernel command line. I think an implication of this is that systems haven't been well-tested with interrupts enabled. In general when we've found a firmware issue in one place it ends up happening elsewhere as well, so it wouldn't surprise me if there are other machines that will also be unhappy with interrupts enabled. Would it be possible to automatically detect this case (eg, if we get more than a certain number of interrupts in a certain timeframe immediately after enabling the interrupt) and automatically fall back to polling in that case? It would also mean that users with fixed firmware wouldn't need to pass a parameter.
[PATCH] docs: lkdtm: Modernize and improve details
The details on using LKDTM were overly obscure. Modernize the details and expand examples to better illustrate how to use the interfaces. Additionally add missing SPDX header. Signed-off-by: Kees Cook --- .../fault-injection/provoke-crashes.rst | 56 +++ 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/Documentation/fault-injection/provoke-crashes.rst b/Documentation/fault-injection/provoke-crashes.rst index 9279a3e12278..93775bd4e6c8 100644 --- a/Documentation/fault-injection/provoke-crashes.rst +++ b/Documentation/fault-injection/provoke-crashes.rst @@ -1,16 +1,19 @@ -=== -Provoke crashes -=== +.. SPDX-License-Identifier: GPL-2.0 -The lkdtm module provides an interface to crash or injure the kernel at -predefined crashpoints to evaluate the reliability of crash dumps obtained -using different dumping solutions. The module uses KPROBEs to instrument -crashing points, but can also crash the kernel directly without KRPOBE -support. + +Provoking crashes with Linux Kernel Dump Test Module (LKDTM) + +The lkdtm module provides an interface to disrupt (and usually crash) +the kernel at predefined code locations to evaluate the reliability of +the kernel's exception handling and to test crash dumps obtained using +different dumping solutions. The module uses KPROBEs to instrument the +trigger location, but can also trigger the kernel directly without KPROBE +support via debugfs. -You can provide the way either through module arguments when inserting -the module, or through a debugfs interface. +You can select the location of the trigger ("crash point name") and the +type of action ("crash point type") either through module arguments when +inserting the module, or through the debugfs interface. Usage:: @@ -18,31 +21,38 @@ Usage:: [cpoint_count={>0}] recur_count - Recursion level for the stack overflow test. Default is 10. + Recursion level for the stack overflow test. By default this is + dynamically calculated based on kernel configuration, with the + goal of being just large enough to exhaust the kernel stack. The + value can be seen at `/sys/module/lkdtm/parameters/recur_count`. cpoint_name - Crash point where the kernel is to be crashed. It can be + Where in the kernel to trigger the action. It can be one of INT_HARDWARE_ENTRY, INT_HW_IRQ_EN, INT_TASKLET_ENTRY, FS_DEVRW, MEM_SWAPOUT, TIMERADD, SCSI_DISPATCH_CMD, - IDE_CORE_CP, DIRECT + IDE_CORE_CP, or DIRECT cpoint_type Indicates the action to be taken on hitting the crash point. - It can be one of PANIC, BUG, EXCEPTION, LOOP, OVERFLOW, - CORRUPT_STACK, UNALIGNED_LOAD_STORE_WRITE, OVERWRITE_ALLOCATION, - WRITE_AFTER_FREE, + These are numerous, and best queried directly from debugfs. Some + of the common ones are PANIC, BUG, EXCEPTION, LOOP, and OVERFLOW. + See the contents of `/sys/kernel/debug/provoke-crash/DIRECT` for + a complete list. cpoint_count Indicates the number of times the crash point is to be hit - to trigger an action. The default is 10. + before triggering the action. The default is 10 (except for + DIRECT, which always fires immediately). You can also induce failures by mounting debugfs and writing the type to -/provoke-crash/. E.g.:: +/provoke-crash/. E.g.:: - mount -t debugfs debugfs /mnt - echo EXCEPTION > /mnt/provoke-crash/INT_HARDWARE_ENTRY + mount -t debugfs debugfs /sys/kernel/debug + echo EXCEPTION > /sys/kernel/debug/provoke-crash/INT_HARDWARE_ENTRY +The special file `DIRECT` will induce the action directly without KPROBE +instrumentation. This mode is the only one available when the module is +built for a kernel without KPROBEs support:: -A special file is `DIRECT` which will induce the crash directly without -KPROBE instrumentation. This mode is the only one available when the module -is built on a kernel without KPROBEs support. + # Instead of having a BUG kill your shell, have it kill "cat": + cat <(echo WRITE_RO) >/sys/kernel/debug/provoke-crash/DIRECT -- 2.25.1
Re: [PATCH] PCI: dwc: Added link up check in map_bus of dw_child_pcie_ops
On Wed, Sep 16, 2020 at 01:41:30PM +0800, Zhiqiang Hou wrote: > From: Hou Zhiqiang > > On NXP Layerscape platforms, it results in SError in the > enumeration of the PCIe controller, which is not connecting > with an Endpoint device. And it doesn't make sense to > enumerate the Endpoints when the PCIe link is down. So this > patch added the link up check to avoid to fire configuration > transactions on link down bus. Lorenzo already applied this, but a couple questions: You call out NXP Layerscape specifically, but doesn't this affect other DWC-based platforms, too? You later mentioned imx6, Kishon mentioned dra7xx, Michael mentioned ls1028a, Naresh mentioned ls2088 (probably both the same as your "NXP Layerscape"). The backtrace below contains a bunch of irrelevant info. The timestamps are pointless. The backtrace past pci_scan_single_device+0x80/0x100 or so really doesn't add anything either. It'd be nice to have a comment in the code because the code *looks* wrong and racy. Without a hint, everybody who sees it will have to dig through the history to see why we tolerate the race. > [0.807773] SError Interrupt on CPU2, code 0xbf02 -- SError > [0.807775] CPU: 2 PID: 1 Comm: swapper/0 Not tainted > 5.9.0-rc5-next-20200914-1-gf965d3ec86fa #67 > [0.807776] Hardware name: LS1046A RDB Board (DT) > [0.80] pstate: 2085 (nzCv daIf -PAN -UAO BTYPE=--) > [0.807778] pc : pci_generic_config_read+0x3c/0xe0 > [0.807778] lr : pci_generic_config_read+0x24/0xe0 > [0.807779] sp : 80001003b7b0 > [0.807780] x29: 80001003b7b0 x28: 80001003ba74 > [0.807782] x27: 000971d96800 x26: 00096e77e0a8 > [0.807784] x25: 80001003b874 x24: 80001003b924 > [0.807786] x23: 0004 x22: > [0.807788] x21: x20: 80001003b874 > [0.807790] x19: 0004 x18: > [0.807791] x17: 00c0 x16: fe0025981840 > [0.807793] x15: b94c75b69948 x14: 62203a383634203a > [0.807795] x13: 666e6f635f726568 x12: 202c31203d207265 > [0.807797] x11: 626d756e3e2d7375 x10: 656877202c307830 > [0.807799] x9 : 203d206e66766564 x8 : 0908 > [0.807801] x7 : 0908 x6 : 80001090 > [0.807802] x5 : 00096e77e080 x4 : > [0.807804] x3 : 0003 x2 : 84fa3440ff7e7000 > [0.807806] x1 : x0 : 800010034000 > [0.807808] Kernel panic - not syncing: Asynchronous SError Interrupt > [0.807809] CPU: 2 PID: 1 Comm: swapper/0 Not tainted > 5.9.0-rc5-next-20200914-1-gf965d3ec86fa #67 > [0.807810] Hardware name: LS1046A RDB Board (DT) > [0.807811] Call trace: > [0.807812] dump_backtrace+0x0/0x1c0 > [0.807813] show_stack+0x18/0x28 > [0.807814] dump_stack+0xd8/0x134 > [0.807814] panic+0x180/0x398 > [0.807815] add_taint+0x0/0xb0 > [0.807816] arm64_serror_panic+0x78/0x88 > [0.807817] do_serror+0x68/0x180 > [0.807818] el1_error+0x84/0x100 > [0.807818] pci_generic_config_read+0x3c/0xe0 > [0.807819] dw_pcie_rd_other_conf+0x78/0x110 > [0.807820] pci_bus_read_config_dword+0x88/0xe8 > [0.807821] pci_bus_generic_read_dev_vendor_id+0x30/0x1b0 > [0.807822] pci_bus_read_dev_vendor_id+0x4c/0x78 > [0.807823] pci_scan_single_device+0x80/0x100 > [0.807824] pci_scan_slot+0x38/0x130 > [0.807825] pci_scan_child_bus_extend+0x54/0x2a0 > [0.807826] pci_scan_child_bus+0x14/0x20 > [0.807827] pci_scan_bridge_extend+0x230/0x570 > [0.807828] pci_scan_child_bus_extend+0x134/0x2a0 > [0.807829] pci_scan_root_bus_bridge+0x64/0xf0 > [0.807829] pci_host_probe+0x18/0xc8 > [0.807830] dw_pcie_host_init+0x220/0x378 > [0.807831] ls_pcie_probe+0x104/0x140 > [0.807832] platform_drv_probe+0x54/0xa8 > [0.807833] really_probe+0x118/0x3e0 > [0.807834] driver_probe_device+0x5c/0xc0 > [0.807835] device_driver_attach+0x74/0x80 > [0.807835] __driver_attach+0x8c/0xd8 > [0.807836] bus_for_each_dev+0x7c/0xd8 > [0.807837] driver_attach+0x24/0x30 > [0.807838] bus_add_driver+0x154/0x200 > [0.807839] driver_register+0x64/0x120 > [0.807839] __platform_driver_probe+0x7c/0x148 > [0.807840] ls_pcie_driver_init+0x24/0x30 > [0.807841] do_one_initcall+0x60/0x1d8 > [0.807842] kernel_init_freeable+0x1f4/0x24c > [0.807843] kernel_init+0x14/0x118 > [0.807843] ret_from_fork+0x10/0x34 > [0.807854] SMP: stopping secondary CPUs > [0.807855] Kernel Offset: 0x394c6408 from 0x80001000 > [0.807856] PHYS_OFFSET: 0x8bfd4000 > [0.807856] CPU features: 0x0240022,21806000 > [0.807857] Memory Limit: none > > Fixes: c2b0c098fbd1 ("PCI: dwc: Use generic config accessors") > Signed-off-by: Hou Zhiqiang > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/dri
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On 14:10 Thu 15 Oct 2020, Joe Perches wrote: On Thu, 2020-10-15 at 19:24 +0530, Bhaskar Chowdhury wrote: On 06:38 Thu 15 Oct 2020, Joe Perches wrote: > On Thu, 2020-10-15 at 18:53 +0530, Bhaskar Chowdhury wrote: > > Fix the space in the middle in below entry. > > > > memry||memory > [] > > diff --git a/scripts/spelling.txt b/scripts/spelling.txt > [] > > @@ -885,7 +885,7 @@ meetign||meeting > > memeory||memory > > memmber||member > > memoery||memory > > -memry ||memory > > +memry||memory > > No. Don't post a bad patch, assume > it's applied and then post a fix to > the bad patch as v2. > > Send a single clean patch. > Not sure what you mean...could you elaborate...don't know what is going on..> You sent a patch with a defect Who doesn't??? You sent a V2 patch that just corrects the defect in the first patch. That's how it is working here for long time ...I am not sure about your involvement. Instead send a v3 patch without any defect. No point ...I think your understanding takes a back seat...could you please rebrush it?? ..and you are telling me something which isn't practice here ..maybe some other project with some other people you worked with... certainly not here ... Long story short, you found a flaw what I sent and I appreciate your review and corrected the trivialities ...but rest what you are asking is garbage . More ...I don't know why I am explaining this stuff to you...we have been here doing this stuff for a long time ... (not sure about you) Versioning is important for the person who maintain that file..because of lots of reason ...and I am sure either you have forgotten or it skipped your mind for the moment ..that is why I suggest ...please rebrush your understanding ... Please don't unnecessarily streach thing ...we have other things to do ...do not bringing "new rules" here for the sake of it. signature.asc Description: PGP signature
Re: [PATCH v11 1/4] bitops: Introduce the for_each_set_clump macro
On Tue, Oct 6, 2020 at 4:56 PM Andy Shevchenko wrote: > > On Tue, Oct 06, 2020 at 02:52:16PM +0530, Syed Nayyar Waris wrote: > > This macro iterates for each group of bits (clump) with set bits, > > within a bitmap memory region. For each iteration, "start" is set to > > the bit offset of the found clump, while the respective clump value is > > stored to the location pointed by "clump". Additionally, the > > bitmap_get_value() and bitmap_set_value() functions are introduced to > > respectively get and set a value of n-bits in a bitmap memory region. > > The n-bits can have any size less than or equal to BITS_PER_LONG. > > Moreover, during setting value of n-bit in bitmap, if a situation arise > > that the width of next n-bit is exceeding the word boundary, then it > > will divide itself such that some portion of it is stored in that word, > > while the remaining portion is stored in the next higher word. Similar > > situation occurs while retrieving the value from bitmap. > > ... > > > @@ -75,7 +75,11 @@ > > * bitmap_from_arr32(dst, buf, nbits) Copy nbits from u32[] buf > > to dst > > * bitmap_to_arr32(buf, src, nbits)Copy nbits from buf to > > u32[] dst > > * bitmap_get_value8(map, start) Get 8bit value from map at > > start > > + * bitmap_get_value(map, start, nbits) Get bit value of size > > + * 'nbits' from map at start > > * bitmap_set_value8(map, value, start)Set 8bit value to map at > > start > > + * bitmap_set_value(map, value, start, nbits) Set bit value of size > > 'nbits' > > + * of map at start > > Formatting here is done with solely spaces, no TABs. Okay. Done > > ... > > > +/** > > + * bitmap_get_value - get a value of n-bits from the memory region > > + * @map: address to the bitmap memory region > > + * @start: bit offset of the n-bit value > > + * @nbits: size of value in bits (must be between 1 and BITS_PER_LONG > > inclusive). > > > > + * nbits less than 1 or more than BITS_PER_LONG causes undefined > > behaviour. > > Please, detach this from field description and move to a main description. Okay. Done. > > > + * > > + * Returns value of nbits located at the @start bit offset within the @map > > + * memory region. > > + */ > > ... > > > + return (map[index] >> offset) & GENMASK(nbits - 1, 0); > > Have you considered to use rather BIT{_ULL}(nbits) - 1? > It maybe better for code generation. Yes I have considered using BIT{_ULL} in earlier versions of patchset. It has a problem: This macro when used in both bitmap_get_value and bitmap_set_value functions, it will give unexpected results when nbits or clump size is BITS_PER_LONG (32 or 64 depending on arch). Actually when nbits (clump size) is 64 (BITS_PER_LONG is 64, for example), (BIT(nbits) - 1) gives a value of zero and when this zero is ANDed with any value, it makes it full zero. This is unexpected, and incorrect calculation occurs. What actually happens is in the macro expansion of BIT(64), that is 1 << 64, the '1' overflows from leftmost bit position (most significant bit) and re-enters at the rightmost bit position (least significant bit), therefore 1 << 64 becomes '0x1', and when another '1' is subtracted from this, the final result becomes 0. This is undefined behavior in the C standard (section 6.5.7 in the N1124) > > ... > > > +/** > > + * bitmap_set_value - set n-bit value within a memory region > > + * @map: address to the bitmap memory region > > + * @value: value of nbits > > + * @start: bit offset of the n-bit value > > + * @nbits: size of value in bits (must be between 1 and BITS_PER_LONG > > inclusive). > > > + * nbits less than 1 or more than BITS_PER_LONG causes undefined > > behaviour. > > Please, detach this from field description and move to a main description. Okay. Done > > > + */ > > ... > > > + value &= GENMASK(nbits - 1, 0); > > Ditto. > > > + map[index] &= ~(GENMASK(nbits + offset - 1, offset)); > > Last time I checked such GENMASK) use, it gave a lot of code when > GENMASK(nbits - 1, 0) << offset works much better, but see also above. Yes I have incorporated your suggestion to use the '<<' operator. Thank You. > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On Fri, 2020-10-16 at 04:19 +0530, Bhaskar Chowdhury wrote: > On 14:10 Thu 15 Oct 2020, Joe Perches wrote: > > On Thu, 2020-10-15 at 19:24 +0530, Bhaskar Chowdhury wrote: > > > On 06:38 Thu 15 Oct 2020, Joe Perches wrote: > > > > On Thu, 2020-10-15 at 18:53 +0530, Bhaskar Chowdhury wrote: > > > > > Fix the space in the middle in below entry. > > > > > > > > > > memry||memory > > > > [] > > > > > diff --git a/scripts/spelling.txt b/scripts/spelling.txt > > > > [] > > > > > @@ -885,7 +885,7 @@ meetign||meeting > > > > > memeory||memory > > > > > memmber||member > > > > > memoery||memory > > > > > -memry ||memory > > > > > +memry||memory > > > > > > > > No. Don't post a bad patch, assume > > > > it's applied and then post a fix to > > > > the bad patch as v2. > > > > > > > > Send a single clean patch. > > > > > > > > > > Not sure what you mean...could you elaborate...don't know what is going > > > on..> > > > > You sent a patch with a defect > > Who doesn't??? No one. > > You sent a V2 patch that just corrects the defect in the first patch. > > That's how it is working here for long time ...I am not sure about your > involvement. wrong. Your first patch has not been, and should not be applied, as it has a trivial known defect. > Please don't unnecessarily streach thing ...we have other things to do ...do > not bringing "new rules" here for the sake of it. It's not a new rule. Don't introduce patches with defects.
[GIT PULL] f2fs update for 5.10-rc1
Hi Linus, Could you please consider this pull request? Thanks, The following changes since commit 581cb3a26baf846ee9636214afaa5333919875b1: Merge tag 'f2fs-for-5.9-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs (2020-09-10 13:12:46 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git tags/f2fs-for-5.10-rc1 for you to fetch changes up to 788e96d1d39949fc91457a816f4bda0d374c257b: f2fs: code cleanup by removing unnecessary check (2020-10-14 13:23:41 -0700) f2fs-for-5.10-rc1 In this round, we've added new features such as zone capacity for ZNS and a new GC policy, ATGC, along with in-memory segment management. In addition, we could improve the decompression speed significantly by changing virtual mapping method. Even though we've fixed lots of small bugs in compression support, I feel that it becomes more stable so that I could give it a try in production. Enhancement: - suport zone capacity in NVMe Zoned Namespace devices - introduce in-memory current segment management - add standart casefolding support - support age threshold based garbage collection - improve decompression speed by changing virtual mapping method Bug fix: - fix condition checks in some ioctl() such as compression, move_range, etc - fix 32/64bits support in data structures - fix memory allocation in zstd decompress - add some boundary checks to avoid kernel panic on corrupted image - fix disallowing compression for non-empty file - fix slab leakage of compressed block writes In addition, it includes code refactoring for better readability and minor bug fixes for compression and zoned device support. Aravind Ramesh (1): f2fs: support zone capacity less than zone size Chao Yu (24): f2fs: compress: remove unneeded code f2fs: introduce inmem curseg f2fs: record average update time of segment f2fs: inherit mtime of original block during GC f2fs: support 64-bits key in f2fs rb-tree node entry f2fs: fix compile warning f2fs: compress: use more readable atomic_t type for {cic,dic}.ref f2fs: support age threshold based garbage collection f2fs: allocate proper size memory for zstd decompress f2fs: ignore compress mount option on image w/o compression feature f2fs: trace: fix typo f2fs: clean up kvfree f2fs: do sanity check on zoned block device path f2fs: relocate blkzoned feature check f2fs: remove unneeded parameter in find_in_block() f2fs: fix uninit-value in f2fs_lookup f2fs: fix to check segment boundary during SIT page readahead f2fs: fix to do sanity check on segment/section count f2fs: compress: introduce page array slab cache f2fs: compress: introduce cic/dic slab cache f2fs: compress: fix to disallow enabling compress on non-empty file f2fs: fix to set SBI_NEED_FSCK flag for inconsistent inode f2fs: don't issue flush in f2fs_flush_device_cache() for nobarrier case f2fs: introduce check_swap_activate_fast() Chengguang Xu (1): f2fs: code cleanup by removing unnecessary check Daeho Jeong (6): f2fs: add block address limit check to compressed file f2fs: change compr_blocks of superblock info to 64bit f2fs: change i_compr_blocks of inode to atomic value f2fs: change return value of f2fs_disable_compressed_file to bool f2fs: change virtual mapping way for compression pages f2fs: fix writecount false positive in releasing compress blocks Dan Robertson (1): f2fs: check position in move range ioctl Daniel Rosenberg (3): unicode: Add utf8_casefold_hash fs: Add standard casefolding support f2fs: Use generic casefolding support Eric Biggers (1): f2fs: reject CASEFOLD inode flag without casefold feature Jack Qiu (1): f2fs: correct statistic of APP_DIRECT_IO/APP_DIRECT_READ_IO Jaegeuk Kim (4): f2fs: point man pages for some f2fs utils f2fs: fix slab leak of rpages pointer f2fs: fix memory alignment to support 32bit f2fs: handle errors of f2fs_get_meta_page_nofail Jamie Iles (1): f2fs: wait for sysfs kobject removal before freeing f2fs_sb_info Matthew Wilcox (Oracle) (1): f2fs: Simplify SEEK_DATA implementation Randy Dunlap (1): f2fs: Documentation edits/fixes Wang Xiaojun (3): f2fs: remove unused check on version_bitmap f2fs: remove duplicated code in sanity_check_area_boundary f2fs: fix wrong total_sections check and fsmeta check Xiaojun Wang (2): f2fs: remove duplicated type casting f2fs: change return value of reserved_segments to unsigned int Zhang Qilong (1): f2fs: add trace exit in exception path Documentation/ABI/testing/sysfs-fs-f2fs | 3 +- Documentation/filesystems/f2fs.rst | 82 -
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On 15:53 Thu 15 Oct 2020, Joe Perches wrote: On Fri, 2020-10-16 at 04:19 +0530, Bhaskar Chowdhury wrote: On 14:10 Thu 15 Oct 2020, Joe Perches wrote: > On Thu, 2020-10-15 at 19:24 +0530, Bhaskar Chowdhury wrote: > > On 06:38 Thu 15 Oct 2020, Joe Perches wrote: > > > On Thu, 2020-10-15 at 18:53 +0530, Bhaskar Chowdhury wrote: > > > > Fix the space in the middle in below entry. > > > > > > > > memry||memory > > > [] > > > > diff --git a/scripts/spelling.txt b/scripts/spelling.txt > > > [] > > > > @@ -885,7 +885,7 @@ meetign||meeting > > > > memeory||memory > > > > memmber||member > > > > memoery||memory > > > > -memry ||memory > > > > +memry||memory > > > > > > No. Don't post a bad patch, assume > > > it's applied and then post a fix to > > > the bad patch as v2. > > > > > > Send a single clean patch. > > > > > > > Not sure what you mean...could you elaborate...don't know what is going on..> > > You sent a patch with a defect Who doesn't??? No one. > You sent a V2 patch that just corrects the defect in the first patch. That's how it is working here for long time ...I am not sure about your involvement. wrong. Your first patch has not been, and should not be applied, as it has a trivial known defect. Please don't unnecessarily streach thing ...we have other things to do ...do not bringing "new rules" here for the sake of it. It's not a new rule. Don't introduce patches with defects. You have all flawed understanding...please stay away ..if you don't understand something...> signature.asc Description: PGP signature
Re: [RFC PATCH 2/3] hugetlbfs: introduce hinode_rwsem for pmd sharing synchronization
On Tue, Oct 13, 2020 at 04:10:59PM -0700, Mike Kravetz wrote: > Due to pmd sharing, the huge PTE pointer returned by huge_pte_alloc > may not be valid. This can happen if a call to huge_pmd_unshare for > the same pmd is made in another thread. > > To address this issue, add a rw_semaphore (hinode_rwsem) to the hugetlbfs > inode. > - hinode_rwsem is taken in read mode before calling huge_pte_alloc, and > held until finished with the returned pte pointer. > - hinode_rwsem is held in write mode whenever huge_pmd_unshare is called. > > In the locking hierarchy, hinode_rwsem must be taken before a page lock. > > In an effort to minimize performance impacts, hinode_rwsem is not taken > if the caller knows the target can not possibly be part of a shared pmd. > lockdep_assert calls are added to huge_pmd_share and huge_pmd_unshare to > help catch callers not using the proper locking. > > Signed-off-by: Mike Kravetz Hi Mike, I didn't find a problem on main idea of introducing hinode_rwsem, so I'm fine if the known problems are fixed. I have one question. This patch seems to make sure that huge_pmd_unshare() are called under holding hinode_rwsem in write mode for some case. Some callers of try_to_unmap() seem not to hold it like shrink_page_list(), unmap_page(), which is OK because they never call try_to_unmap() for hugetlb pages. And unmap_ref_private() doesn't takes hinode_rwsem either, and that's also OK because this function never handles pmd sharing case. So what about unmap_single_vma()? It seems that this generic function could reach huge_pmd_unshare() without hinode_rwsem, so what prevents the race here? (Maybe I might miss some assumption or condition over this race...) I left a few other minor comments below ... > @@ -4424,6 +4442,11 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct > vm_area_struct *vma, > > ptep = huge_pte_offset(mm, haddr, huge_page_size(h)); > if (ptep) { > + /* > + * Since we hold no locks, ptep could be stale. That is > + * OK as we are only making decisions based on content and > + * not actually modifying content here. > + */ nice comment, thank you. > entry = huge_ptep_get(ptep); > if (unlikely(is_hugetlb_entry_migration(entry))) { > migration_entry_wait_huge(vma, mm, ptep); > @@ -4431,20 +4454,32 @@ vm_fault_t hugetlb_fault(struct mm_struct *mm, struct > vm_area_struct *vma, > } else if (unlikely(is_hugetlb_entry_hwpoisoned(entry))) > return VM_FAULT_HWPOISON_LARGE | > VM_FAULT_SET_HINDEX(hstate_index(h)); > - } else { > - ptep = huge_pte_alloc(mm, haddr, huge_page_size(h)); > - if (!ptep) > - return VM_FAULT_OOM; > } > > + /* > + * Acquire hinode_sem before calling huge_pte_alloc and hold hinode_rwsem? > + * until finished with ptep. This prevents huge_pmd_unshare from > + * being called elsewhere and making the ptep no longer valid. > + * > + * ptep could have already be assigned via huge_pte_offset. That > + * is OK, as huge_pte_alloc will return the same value unless > + * something has changed. > + */ ... > @@ -278,10 +278,14 @@ static __always_inline ssize_t > __mcopy_atomic_hugetlb(struct mm_struct *dst_mm, > BUG_ON(dst_addr >= dst_start + len); > > /* > - * Serialize via hugetlb_fault_mutex > + * Serialize via hinode_rwsem hugetlb_fault_mutex. ^ "and" here? > + * hinode_rwsem ensures the dst_pte remains valid even > + * in the case of shared pmds. fault mutex prevents > + * races with other faulting threads. >*/ > idx = linear_page_index(dst_vma, dst_addr); > mapping = dst_vma->vm_file->f_mapping; > + hinode_lock_read(mapping, dst_vma, dst_addr); > hash = hugetlb_fault_mutex_hash(mapping, idx); > mutex_lock(&hugetlb_fault_mutex_table[hash]); Thanks, Naoya Horiguchi
Re: [GIT PULL] SafeSetID changes for v5.10
These were rebased since the merge window started, for no apparent reason. Were they in linux-next? And if so, why was I sent some different version? Linus
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On Fri, 2020-10-16 at 04:25 +0530, Bhaskar Chowdhury wrote: > You have all flawed understanding...please stay away .. > if you don't understand something... You're funny. You're wrong, but you're still funny.
Re: [PATCH V2] scripts: spelling: Remove space in the entry memry to memory
On 16:06 Thu 15 Oct 2020, Joe Perches wrote: On Fri, 2020-10-16 at 04:25 +0530, Bhaskar Chowdhury wrote: You have all flawed understanding...please stay away .. if you don't understand something... You're funny. You're wrong, but you're still funny. ROFL ..you too...what a waste of time ...shame that I am engage this kind of conversation ...heck signature.asc Description: PGP signature
Re: [GIT PULL] parisc architecture updates for kernel v5.10-rc1
The pull request you sent on Thu, 15 Oct 2020 09:20:25 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git > parisc-5.10-1 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/7286d2a37eb955c5eeec2b042844f1c1b3ff0fe1 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] tracing: Updates for 5.10
The pull request you sent on Thu, 15 Oct 2020 13:53:45 -0400: > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git > trace-v5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/fefa636d815975b34afc45f50852a2810fb23ba9 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] integrity subsystem updates for v5.10
The pull request you sent on Wed, 14 Oct 2020 13:19:31 -0400: > git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git > tags/integrity-v5.10 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/840e5bb326bbcb16ce82dd2416d2769de4839aea Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] Hyper-V commits for 5.10, part 2
The pull request you sent on Thu, 15 Oct 2020 16:32:48 +: > ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git > tags/hyperv-next-signed has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/2d0f6b0aab9afbd6fdf3514cb4acc249d7aebf9c Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
[PATCH] mmc: sdhci-of-esdhc: set timeout to max before tuning
On rare occations there is the following error: mmc0: Tuning timeout, falling back to fixed sampling clock There are SD cards which takes a significant longer time to reply to the first CMD19 command. The eSDHC takes the data timeout value into account during the tuning period. The SDHCI core doesn't explicitly set this timeout for the tuning procedure. Thus on the slow cards, there might be a spurious "Buffer Read Ready" interrupt, which in turn triggers a wrong sequence of events. In the end this will lead to an unsuccessful tuning procedure and to the above error. To workaround this, set the timeout to the maximum value (which is the best we can do) and the SDHCI core will take care of the proper timeout handling. Signed-off-by: Michael Walle --- drivers/mmc/host/sdhci-of-esdhc.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index 0b45eff6fed4..baf7801a1804 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -1052,6 +1052,17 @@ static int esdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) esdhc_tuning_block_enable(host, true); + /* +* The eSDHC controller takes the data timeout value into account +* during tuning. If the SD card is too slow sending the response, the +* timer will expire and a "Buffer Read Ready" interrupt without data +* is triggered. This leads to tuning errors. +* +* Just set the timeout to the maximum value because the core will +* already take care of it in sdhci_send_tuning(). +*/ + sdhci_writeb(host, 0xe, SDHCI_TIMEOUT_CONTROL); + hs400_tuning = host->flags & SDHCI_HS400_TUNING; do { -- 2.20.1
Re: linux-next: manual merge of the char-misc tree with the powerpc tree
Hi all, On Tue, 6 Oct 2020 18:35:06 +1100 Stephen Rothwell wrote: > > Today's linux-next merge of the char-misc tree got a conflict in: > > drivers/misc/ocxl/Kconfig > > between commit: > > dde6f18a8779 ("ocxl: Don't return trigger page when allocating an > interrupt") > > from the powerpc tree and commit: > > 4b53a3c72116 ("ocxl: fix kconfig dependency warning for OCXL") > > from the char-misc tree. > > I fixed it up (see below) and can carry the fix as necessary. This > is now fixed as far as linux-next is concerned, but any non trivial > conflicts should be mentioned to your upstream maintainer when your tree > is submitted for merging. You may also want to consider cooperating > with the maintainer of the conflicting tree to minimise any particularly > complex conflicts. > > diff --cc drivers/misc/ocxl/Kconfig > index 0d815b2a40b3,947294f6d7f4.. > --- a/drivers/misc/ocxl/Kconfig > +++ b/drivers/misc/ocxl/Kconfig > @@@ -9,9 -9,8 +9,9 @@@ config OCXL_BAS > > config OCXL > tristate "OpenCAPI coherent accelerator support" > -depends on PPC_POWERNV && PCI && EEH && HOTPLUG_PCI_POWERNV > +depends on PPC_POWERNV && PCI && EEH && PPC_XIVE_NATIVE > ++depends on HOTPLUG_PCI_POWERNV > select OCXL_BASE > - select HOTPLUG_PCI_POWERNV > default m > help > Select this option to enable the ocxl driver for Open This is now a conflict between the powerpc tree and Linus' tree. -- Cheers, Stephen Rothwell pgpxqie4XDWCo.pgp Description: OpenPGP digital signature
Re: cgroup and FALLOC_FL_PUNCH_HOLE: WARNING: CPU: 13 PID: 2438 at mm/page_counter.c:57 page_counter_uncharge+0x4b/0x5
On 10/14/20 11:31 AM, Mike Kravetz wrote: > On 10/14/20 11:18 AM, David Hildenbrand wrote: > > FWIW - I ran libhugetlbfs tests which do a bunch of hole punching > with (and without) hugetlb controller enabled and did not see this issue. > I took a closer look after running just the fallocate_stress test in libhugetlbfs. Here are the cgroup counter values: hugetlb.2MB.failcnt 0 hugetlb.2MB.limit_in_bytes 9223372036854771712 hugetlb.2MB.max_usage_in_bytes 209715200 hugetlb.2MB.rsvd.failcnt 0 hugetlb.2MB.rsvd.limit_in_bytes 9223372036854771712 hugetlb.2MB.rsvd.max_usage_in_bytes 601882624 hugetlb.2MB.rsvd.usage_in_bytes 392167424 hugetlb.2MB.usage_in_bytes 0 We did not hit the WARN_ON_ONCE(), but the 'rsvd.usage_in_bytes' value is not correct in that it should be zero. No huge page reservations remain after the test. HugePages_Total:1024 HugePages_Free: 1024 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB Hugetlb: 2097152 kB To try and better understand the reservation cgroup controller, I addded a few printks to the code. While running fallocate_stress with the printks, I can consistently hit the WARN_ON_ONCE() due to the counter going negative. Here are the cgroup counter values after such a run: hugetlb.2MB.failcnt 0 hugetlb.2MB.limit_in_bytes 9223372036854771712 hugetlb.2MB.max_usage_in_bytes 209715200 hugetlb.2MB.rsvd.failcnt 3 hugetlb.2MB.rsvd.limit_in_bytes 9223372036854771712 hugetlb.2MB.rsvd.max_usage_in_bytes 251658240 hugetlb.2MB.rsvd.usage_in_bytes 18446744073487253504 hugetlb.2MB.usage_in_bytes 0 Again, no reserved pages after the test. HugePages_Total:1024 HugePages_Free: 1024 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB Hugetlb: 2097152 kB I have some basic hugetlb hole punch functionality tests. Running these on the kernel with added printk's does not cause any issues. In order to reproduce, I need to run fallocate_stress test which will cause hole punch to race with page fault. Best guess at this time is that some of the error/race detection reservation back out code is not properly dealing with cgroup accounting. I'll take a look at this as well. -- Mike Kravetz
Re: [PATCH 0/7] TC-ETF support PTP clocks series
Andreas, On Wed, Oct 14 2020 at 09:12, Andreas Meisinger wrote: > Sorry about the wrong format/style of my last mail, hope I did get it > right this time. No problem. Yes this looks better. The only thing which could be improved is that your mail client fails to add In-Reply-To: References: ... headers and instead has the MS lookout specific Thread-Topic: [PATCH 0/7] TC-ETF support PTP clocks series Thread-Index: AdaiB8a+x+RhfhtwSZ+NKfvRdyiJkw== headers. If you look at the lore archive you see the effect: https://lore.kernel.org/r/am0pr10mb3073f9694ecad4f612a86fa7fa...@am0pr10mb3073.eurprd10.prod.outlook.com and that happens to all mail clients which use threading based on the standard headers. There is config knob in lookout to enable them. > Let me first point at an important thing because we did have > discussions here about it too. As of the manpages Linux CLOCK_TAI > seems to be defined as an nonsettable clock which does have the same > behaviour as of international atomic time TAI. Yet if it's nonsettable > it can't be linked or synchronized to the value of International > Atomic Time? > > On the other hand there seems to be code in place to change the > CLOCK_TAI offset thus making it basically sort of "setable"? It obviously needs to be modifiable in some way, otherwise synchronization to a master clock via PTP would not work at all. But it cannot be set in the way of settimeofday()/clock_settime() like CLOCK_REALTIME. >> The user space daemon which does the correlation between these PTP >> domains and TAI is required in any case, so the magic clock >> TAI_PRIVATE is not having any advantage. > I think a userspace daemon handling the translation information > between different clocks would be fine. I think it's not really that > relevant who exactly does apply the translation. Kernel level might be > a little bit more precise but I guess it'd depend on other factors > too. Not really. The kernel still provides the timestamp pairs/triplets in the best way the underlying hardware provides it. Some hardware can even provide hardware assistet pairs of ART and PTP clock. > Yet all translation based approaches have in common, setting a clock, > renders translations done in past invalid. It would be required to fix > old translated values along with setting the clock. I assume we > couldn't distinguish between "translated" values and genuine values > after translation, so fixing them might not be possible at all. CLOCK_TAI is not really set after the initial sychronization. It's frequency corrected without causing jumps. PTP daemon uses a PLL based algorithm for that. Of course this adjustment has side effects for translation. > In our usecase we do have a clock for network operation which can't be > set. We can guarantee this because we are able to define the network > not being operational when the defined time is not available 😉. > Having this defined the remaining option would be the target clock to > be set. As of your suggestion that would be CLOCK_TAI. So at this > point "setable" or "nonsettable" would become important. Here > "setable" would introduce a dependency between the clocks. Being > independent from clocks outside of our control was exactly the reason > to introduce the separate network clock. To me this means if CLOCK_TAI > could be changed by anything it couldn't be the base clock for our > usecase if it can't it might be a solution. It's under your control as system designer how you operate CLOCK_TAI. If you use the PTP daemon then it will sync CLOCK_TAI to the PTP clock of your choice. If you don't have PTP at all then you can use NTP to sync to a time server, which is less accurate. You can use PPS or just use nothing. The kernel does not care which non-standard base time or frequency you chose. Applications which communicate over network might care if the other side uses a differnet time universe. Log files which start at 1971 might be interesting to analyse against the log file of your other system which starts in 2020. >> Depending on the frequency drift between CLOCK_TAI and clock PTP/$N >> the timer expiry might be slightly inaccurate, but surely not more >> inaccurate than if that conversion is done purely in user space. >> >> The self rearming posix timers would work too, but the self rearming >> is based on CLOCK_TAI, so rounding errors and drift would be >> accumulative. So I'd rather stay away from them. > > As of the time ranges typically used in tsn networks the drift error > for single shot timers most likely isn't a big deal. But as you > suggest I'd stay away from long running timers as well rearming ones > too, I guess they'd be mostly unusable. Depends. It's a matter of hardware properties, system requirements, application/system designers decisions. So what you consider unusable for your system might be perfectly fine for others. If we add support for this type of correlation then of course these things need to be documented. > Rig
[PATCH] docs: deprecated.rst: Expand str*cpy() replacement notes
The notes on replacing the deprecated str*cpy() functions didn't call enough attention to the change in return type. Add these details and clean up the language a bit more. Signed-off-by: Kees Cook --- Documentation/process/deprecated.rst | 44 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst index ff71d802b53d..9d83b8db8874 100644 --- a/Documentation/process/deprecated.rst +++ b/Documentation/process/deprecated.rst @@ -106,23 +106,29 @@ NUL or newline terminated. strcpy() -strcpy() performs no bounds checking on the destination -buffer. This could result in linear overflows beyond the -end of the buffer, leading to all kinds of misbehaviors. While -`CONFIG_FORTIFY_SOURCE=y` and various compiler flags help reduce the -risk of using this function, there is no good reason to add new uses of -this function. The safe replacement is strscpy(). +strcpy() performs no bounds checking on the destination buffer. This +could result in linear overflows beyond the end of the buffer, leading to +all kinds of misbehaviors. While `CONFIG_FORTIFY_SOURCE=y` and various +compiler flags help reduce the risk of using this function, there is +no good reason to add new uses of this function. The safe replacement +is strscpy(), though care must be given to any cases where the return +value of strcpy() was used, since strscpy() does not return a pointer to +the destination, but rather a count of non-NUL bytes copied (or negative +errno when it truncates). strncpy() on NUL-terminated strings --- -Use of strncpy() does not guarantee that the destination buffer -will be NUL terminated. This can lead to various linear read overflows -and other misbehavior due to the missing termination. It also NUL-pads the -destination buffer if the source contents are shorter than the destination -buffer size, which may be a needless performance penalty for callers using -only NUL-terminated strings. The safe replacement is strscpy(). -(Users of strscpy() still needing NUL-padding should instead -use strscpy_pad().) +Use of strncpy() does not guarantee that the destination buffer will +be NUL terminated. This can lead to various linear read overflows and +other misbehavior due to the missing termination. It also NUL-pads +the destination buffer if the source contents are shorter than the +destination buffer size, which may be a needless performance penalty +for callers using only NUL-terminated strings. The safe replacement is +strscpy(), though care must be given to any cases where the return value +of strncpy() was used, since strscpy() does not return a pointer to the +destination, but rather a count of non-NUL bytes copied (or negative +errno when it truncates). Any cases still needing NUL-padding should +instead use strscpy_pad(). If a caller is using non-NUL-terminated strings, strncpy() can still be used, but destinations should be marked with the `__nonstring @@ -131,10 +137,12 @@ attribute to avoid future compiler warnings. strlcpy() - -strlcpy() reads the entire source buffer first, possibly exceeding -the given limit of bytes to copy. This is inefficient and can lead to -linear read overflows if a source string is not NUL-terminated. The -safe replacement is strscpy(). +strlcpy() reads the entire source buffer first (since the return value +is meant to match that of strlen()). This read may exceed the destination +size limit. This is both inefficient and can lead to linear read overflows +if a source string is not NUL-terminated. The safe replacement is strscpy(), +though care must be given to any cases where the return value of strlcpy() +is used, since strscpy() will return negative errno values when it truncates. %p format specifier --- -- 2.25.1
[PATCH net-next v2 4/9] net: ethernet: ti: cpsw_ale: add cpsw_ale_vlan_del_modify()
Add/export cpsw_ale_vlan_del_modify() and use it in cpsw_switchdev instead of generic cpsw_ale_del_vlan() to avoid mixing 8021Q and switchdev VLAN offload. This is preparation patch equired by follow up changes. Signed-off-by: Grygorii Strashko --- drivers/net/ethernet/ti/cpsw_ale.c | 24 +--- drivers/net/ethernet/ti/cpsw_ale.h | 1 + drivers/net/ethernet/ti/cpsw_switchdev.c | 2 +- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c index a6a455c32628..b1cce39eda17 100644 --- a/drivers/net/ethernet/ti/cpsw_ale.c +++ b/drivers/net/ethernet/ti/cpsw_ale.c @@ -634,8 +634,8 @@ int cpsw_ale_add_vlan(struct cpsw_ale *ale, u16 vid, int port_mask, int untag, return 0; } -static void cpsw_ale_del_vlan_modify(struct cpsw_ale *ale, u32 *ale_entry, -u16 vid, int port_mask) +static void cpsw_ale_vlan_del_modify_int(struct cpsw_ale *ale, u32 *ale_entry, +u16 vid, int port_mask) { int reg_mcast, unreg_mcast; int members, untag; @@ -644,6 +644,7 @@ static void cpsw_ale_del_vlan_modify(struct cpsw_ale *ale, u32 *ale_entry, ALE_ENT_VID_MEMBER_LIST); members &= ~port_mask; if (!members) { + cpsw_ale_set_vlan_untag(ale, ale_entry, vid, 0); cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE); return; } @@ -673,6 +674,23 @@ static void cpsw_ale_del_vlan_modify(struct cpsw_ale *ale, u32 *ale_entry, ALE_ENT_VID_MEMBER_LIST, members); } +int cpsw_ale_vlan_del_modify(struct cpsw_ale *ale, u16 vid, int port_mask) +{ + u32 ale_entry[ALE_ENTRY_WORDS] = {0, 0, 0}; + int idx; + + idx = cpsw_ale_match_vlan(ale, vid); + if (idx < 0) + return -ENOENT; + + cpsw_ale_read(ale, idx, ale_entry); + + cpsw_ale_vlan_del_modify_int(ale, ale_entry, vid, port_mask); + cpsw_ale_write(ale, idx, ale_entry); + + return 0; +} + int cpsw_ale_del_vlan(struct cpsw_ale *ale, u16 vid, int port_mask) { u32 ale_entry[ALE_ENTRY_WORDS] = {0, 0, 0}; @@ -685,7 +703,7 @@ int cpsw_ale_del_vlan(struct cpsw_ale *ale, u16 vid, int port_mask) cpsw_ale_read(ale, idx, ale_entry); if (port_mask) { - cpsw_ale_del_vlan_modify(ale, ale_entry, vid, port_mask); + cpsw_ale_vlan_del_modify_int(ale, ale_entry, vid, port_mask); } else { cpsw_ale_set_vlan_untag(ale, ale_entry, vid, 0); cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE); diff --git a/drivers/net/ethernet/ti/cpsw_ale.h b/drivers/net/ethernet/ti/cpsw_ale.h index 5e4a69662c5f..13fe47687fde 100644 --- a/drivers/net/ethernet/ti/cpsw_ale.h +++ b/drivers/net/ethernet/ti/cpsw_ale.h @@ -134,6 +134,7 @@ static inline int cpsw_ale_get_vlan_p0_untag(struct cpsw_ale *ale, u16 vid) int cpsw_ale_vlan_add_modify(struct cpsw_ale *ale, u16 vid, int port_mask, int untag_mask, int reg_mcast, int unreg_mcast); +int cpsw_ale_vlan_del_modify(struct cpsw_ale *ale, u16 vid, int port_mask); void cpsw_ale_set_unreg_mcast(struct cpsw_ale *ale, int unreg_mcast_mask, bool add); diff --git a/drivers/net/ethernet/ti/cpsw_switchdev.c b/drivers/net/ethernet/ti/cpsw_switchdev.c index 985a929bb957..29747da5c514 100644 --- a/drivers/net/ethernet/ti/cpsw_switchdev.c +++ b/drivers/net/ethernet/ti/cpsw_switchdev.c @@ -227,7 +227,7 @@ static int cpsw_port_vlan_del(struct cpsw_priv *priv, u16 vid, else port_mask = BIT(priv->emac_port); - ret = cpsw_ale_del_vlan(cpsw->ale, vid, port_mask); + ret = cpsw_ale_vlan_del_modify(cpsw->ale, vid, port_mask); if (ret != 0) return ret; -- 2.17.1
[PATCH net-next v2 2/9] net: ethernet: ti: am65-cpsw: move free desc queue mode selection in pdata
In preparation of adding more multi-port K3 CPSW versions move free descriptor queue mode selection in am65_cpsw_pdata, so it can be selected basing on DT compatibility property. Signed-off-by: Grygorii Strashko --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 4 +++- drivers/net/ethernet/ti/am65-cpsw-nuss.h | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 0ee1c7a5c90f..6cea338df7ad 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -1606,7 +1606,6 @@ static int am65_cpsw_nuss_init_rx_chns(struct am65_cpsw_common *common) }; struct k3_ring_cfg fdqring_cfg = { .elm_size = K3_RINGACC_RING_ELSIZE_8, - .mode = K3_RINGACC_RING_MODE_MESSAGE, .flags = K3_RINGACC_RING_SHARED, }; struct k3_udma_glue_rx_flow_cfg rx_flow_cfg = { @@ -1620,6 +1619,7 @@ static int am65_cpsw_nuss_init_rx_chns(struct am65_cpsw_common *common) rx_flow_cfg.ring_rxfdq0_id = fdqring_id; rx_flow_cfg.rx_cfg.size = max_desc_num; rx_flow_cfg.rxfdq_cfg.size = max_desc_num; + rx_flow_cfg.rxfdq_cfg.mode = common->pdata.fdqring_mode; ret = k3_udma_glue_rx_flow_init(rx_chn->rx_chn, i, &rx_flow_cfg); @@ -2006,11 +2006,13 @@ static const struct soc_device_attribute am65_cpsw_socinfo[] = { static const struct am65_cpsw_pdata am65x_sr1_0 = { .quirks = AM65_CPSW_QUIRK_I2027_NO_TX_CSUM, .ale_dev_id = "am65x-cpsw2g", + .fdqring_mode = K3_RINGACC_RING_MODE_MESSAGE, }; static const struct am65_cpsw_pdata j721e_pdata = { .quirks = 0, .ale_dev_id = "am65x-cpsw2g", + .fdqring_mode = K3_RINGACC_RING_MODE_MESSAGE, }; static const struct of_device_id am65_cpsw_nuss_of_mtable[] = { diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h b/drivers/net/ethernet/ti/am65-cpsw-nuss.h index 9c2186b8eae9..b6f228ddc3a0 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h @@ -11,6 +11,7 @@ #include #include #include +#include #include "am65-cpsw-qos.h" struct am65_cpts; @@ -77,6 +78,7 @@ struct am65_cpsw_rx_chn { struct am65_cpsw_pdata { u32 quirks; + enum k3_ring_mode fdqring_mode; const char *ale_dev_id; }; -- 2.17.1
[PATCH net-next v2 0/9] net: ethernet: ti: am65-cpsw: add multi port support in mac-only mode
Hi This series adds multi-port support in mac-only mode (multi MAC mode) to TI AM65x CPSW driver in preparation for enabling support for multi-port devices, like Main CPSW0 on K3 J721E SoC or future CPSW3g on K3 AM64x SoC. The multi MAC mode is implemented by configuring every enabled port in "mac-only" mode (all ingress packets are sent only to the Host port and egress packets directed to target Ext. Port) and creating separate net_device for every enabled Ext. port. This series does not affect on existing CPSW2g one Ext. Port devices and xmit path changes are done only for multi-port devices by splitting xmit path for one-port and multi-port devices. Patches 1-3: Preparation patches to improve K3 CPSW configuration depending on DT Patches 4-5: Fix VLAN offload for multi MAC mode Patch 6: Fixes CPTS context lose issue during PM runtime transition Patch 7: Fixes TX csum offload for multi MAC mode Patches 8-9: add multi-port support to TI AM65x CPSW changes in v2: - patch 8: xmit path split for one-port and multi-port devices to avoid performance losses - patch 9: fixed the case when Port 1 is disabled - Patch 7: added fix for TX csum offload v1: https://lore.kernel.org/patchwork/cover/1315766/ Grygorii Strashko (9): net: ethernet: ti: am65-cpsw: move ale selection in pdata net: ethernet: ti: am65-cpsw: move free desc queue mode selection in pdata net: ethernet: ti: am65-cpsw: use cppi5_desc_is_tdcm() net: ethernet: ti: cpsw_ale: add cpsw_ale_vlan_del_modify() net: ethernet: ti: am65-cpsw: fix vlan offload for multi mac mode net: ethernet: ti: am65-cpsw: keep active if cpts enabled net: ethernet: ti: am65-cpsw: fix tx csum offload for multi mac mode net: ethernet: ti: am65-cpsw: prepare xmit/rx path for multi-port devices in mac-only mode net: ethernet: ti: am65-cpsw: add multi port support in mac-only mode drivers/net/ethernet/ti/am65-cpsw-nuss.c | 327 +++ drivers/net/ethernet/ti/am65-cpsw-nuss.h | 5 + drivers/net/ethernet/ti/cpsw_ale.c | 41 ++- drivers/net/ethernet/ti/cpsw_ale.h | 1 + drivers/net/ethernet/ti/cpsw_switchdev.c | 2 +- 5 files changed, 251 insertions(+), 125 deletions(-) -- 2.17.1
[PATCH net-next v2 5/9] net: ethernet: ti: am65-cpsw: fix vlan offload for multi mac mode
The VLAN offload for AM65x CPSW2G is implemented using existing ALE APIs, which are also used by legacy CPSW drivers. So, now it always adds current Ext. Port and Host as VLAN members when VLAN is added by 8021Q core (.ndo_vlan_rx_add_vid) and forcibly removes VLAN from ALE table in .ndo_vlan_rx_kill_vid(). This works as for AM65x CPSW2G (which has only one Ext. Port) as for legacy CPSW devices (which can't support same VLAN on more then one Port in multi mac (dual-mac) mode). But it doesn't work for the new J721E and AM64x multi port CPSWxG versions doesn't have such restrictions and allow to offload the same VLAN on any number of ports. Now the attempt to add same VLAN on two (or more) K3 CPSWxG Ports will cause: - VLAN members mask overwrite when VLAN is added - VLAN removal from ALE table when any Port removes VLAN This patch fixes an issue by: - switching to use cpsw_ale_vlan_add_modify() instead of cpsw_ale_add_vlan() when VLAN is added to ALE table, so VLAN members mask will not be overwritten; - Updates cpsw_ale_del_vlan() as: if more than one ext. Port is in VLAN member mask then remove only current port from VLAN member mask else remove VLAN ALE entry Example: add: P1 | P0 (Host) -> members mask: P1 | P0 add: P2 | P0-> members mask: P2 | P1 | P0 rem: P1 | P0-> members mask: P2 | P0 rem: P2 | P0-> members mask: - The VLAN is forcibly removed if port_mask=0 passed to cpsw_ale_del_vlan() to preserve existing legacy CPSW drivers functionality. Signed-off-by: Grygorii Strashko --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 8 +--- drivers/net/ethernet/ti/cpsw_ale.c | 19 +++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 65c5446e324e..fecaf6b8270f 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -241,8 +241,8 @@ static int am65_cpsw_nuss_ndo_slave_add_vid(struct net_device *ndev, if (!vid) unreg_mcast = port_mask; dev_info(common->dev, "Adding vlan %d to vlan filter\n", vid); - ret = cpsw_ale_add_vlan(common->ale, vid, port_mask, - unreg_mcast, port_mask, 0); + ret = cpsw_ale_vlan_add_modify(common->ale, vid, port_mask, + unreg_mcast, port_mask, 0); pm_runtime_put(common->dev); return ret; @@ -252,6 +252,7 @@ static int am65_cpsw_nuss_ndo_slave_kill_vid(struct net_device *ndev, __be16 proto, u16 vid) { struct am65_cpsw_common *common = am65_ndev_to_common(ndev); + struct am65_cpsw_port *port = am65_ndev_to_port(ndev); int ret; if (!netif_running(ndev) || !vid) @@ -264,7 +265,8 @@ static int am65_cpsw_nuss_ndo_slave_kill_vid(struct net_device *ndev, } dev_info(common->dev, "Removing vlan %d from vlan filter\n", vid); - ret = cpsw_ale_del_vlan(common->ale, vid, 0); + ret = cpsw_ale_del_vlan(common->ale, vid, + BIT(port->port_id) | ALE_PORT_HOST); pm_runtime_put(common->dev); return ret; diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c index b1cce39eda17..cdc308a2aa3e 100644 --- a/drivers/net/ethernet/ti/cpsw_ale.c +++ b/drivers/net/ethernet/ti/cpsw_ale.c @@ -694,7 +694,7 @@ int cpsw_ale_vlan_del_modify(struct cpsw_ale *ale, u16 vid, int port_mask) int cpsw_ale_del_vlan(struct cpsw_ale *ale, u16 vid, int port_mask) { u32 ale_entry[ALE_ENTRY_WORDS] = {0, 0, 0}; - int idx; + int members, idx; idx = cpsw_ale_match_vlan(ale, vid); if (idx < 0) @@ -702,11 +702,22 @@ int cpsw_ale_del_vlan(struct cpsw_ale *ale, u16 vid, int port_mask) cpsw_ale_read(ale, idx, ale_entry); - if (port_mask) { - cpsw_ale_vlan_del_modify_int(ale, ale_entry, vid, port_mask); - } else { + /* if !port_mask - force remove VLAN (legacy). +* Check if there are other VLAN members ports +* if no - remove VLAN. +* if yes it means same VLAN was added to >1 port in multi port mode, so +* remove port_mask ports from VLAN ALE entry excluding Host port. +*/ + members = cpsw_ale_vlan_get_fld(ale, ale_entry, ALE_ENT_VID_MEMBER_LIST); + members &= ~port_mask; + + if (!port_mask || !members) { + /* last port or force remove - remove VLAN */ cpsw_ale_set_vlan_untag(ale, ale_entry, vid, 0); cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_FREE); + } else { + port_mask &= ~ALE_PORT_HOST; + cpsw_ale_vlan_del_modify_int(ale, ale_entry, vid, port_mask); } cpsw_ale_write(ale, idx, ale_entry); -- 2.17.1
[PATCH net-next v2 3/9] net: ethernet: ti: am65-cpsw: use cppi5_desc_is_tdcm()
Use cppi5_desc_is_tdcm() helper for teardown indicator detection instead of hard-coded value. Signed-off-by: Grygorii Strashko --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 6cea338df7ad..65c5446e324e 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -767,7 +767,7 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_common *common, return ret; } - if (desc_dma & 0x1) { + if (cppi5_desc_is_tdcm(desc_dma)) { dev_dbg(dev, "%s RX tdown flow: %u\n", __func__, flow_idx); return 0; } @@ -935,7 +935,7 @@ static int am65_cpsw_nuss_tx_compl_packets(struct am65_cpsw_common *common, if (res == -ENODATA) break; - if (desc_dma & 0x1) { + if (cppi5_desc_is_tdcm(desc_dma)) { if (atomic_dec_and_test(&common->tdown_cnt)) complete(&common->tdown_complete); break; -- 2.17.1
[PATCH net-next v2 1/9] net: ethernet: ti: am65-cpsw: move ale selection in pdata
In preparation of adding more multi-port K3 CPSW versions move ALE selection in am65_cpsw_pdata, so it can be selected basing on DT compatibility property. Signed-off-by: Grygorii Strashko --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 4 +++- drivers/net/ethernet/ti/am65-cpsw-nuss.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 501d676fd88b..0ee1c7a5c90f 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -2005,10 +2005,12 @@ static const struct soc_device_attribute am65_cpsw_socinfo[] = { static const struct am65_cpsw_pdata am65x_sr1_0 = { .quirks = AM65_CPSW_QUIRK_I2027_NO_TX_CSUM, + .ale_dev_id = "am65x-cpsw2g", }; static const struct am65_cpsw_pdata j721e_pdata = { .quirks = 0, + .ale_dev_id = "am65x-cpsw2g", }; static const struct of_device_id am65_cpsw_nuss_of_mtable[] = { @@ -2145,7 +2147,7 @@ static int am65_cpsw_nuss_probe(struct platform_device *pdev) ale_params.ale_ageout = AM65_CPSW_ALE_AGEOUT_DEFAULT; ale_params.ale_ports = common->port_num + 1; ale_params.ale_regs = common->cpsw_base + AM65_CPSW_NU_ALE_BASE; - ale_params.dev_id = "am65x-cpsw2g"; + ale_params.dev_id = common->pdata.ale_dev_id; ale_params.bus_freq = common->bus_freq; common->ale = cpsw_ale_create(&ale_params); diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h b/drivers/net/ethernet/ti/am65-cpsw-nuss.h index 993e1d4d3222..9c2186b8eae9 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h @@ -77,6 +77,7 @@ struct am65_cpsw_rx_chn { struct am65_cpsw_pdata { u32 quirks; + const char *ale_dev_id; }; struct am65_cpsw_common { -- 2.17.1
[PATCH net-next v2 8/9] net: ethernet: ti: am65-cpsw: prepare xmit/rx path for multi-port devices in mac-only mode
This patch adds multi-port support to TI AM65x CPSW driver xmit/rx path in preparation for adding support for multi-port devices, like Main CPSW0 on K3 J721E SoC or future CPSW3g on K3 AM64x SoC. Hence DMA channels are common/shared for all ext Ports and the RX/TX NAPI and DMA processing going to be assigned to first available netdev this patch: - ensures all RX descriptors fields are initialized; - adds synchronization for TX DMA push/pop operation (locking) as Networking core locks are not enough any more; - updates TX bql processing for every packet in am65_cpsw_nuss_tx_compl_packets() as every completed TX skb can have different ndev assigned (come from different netdevs). To avoid performance issues for existing one-port CPSW2g devices the above changes are done only for multi-port devices by splitting xmit path for one-port and multi-port devices. Signed-off-by: Grygorii Strashko --- changes in v2: - xmit path split for one-port and multi-port devices to avoid performance losses drivers/net/ethernet/ti/am65-cpsw-nuss.c | 141 +-- drivers/net/ethernet/ti/am65-cpsw-nuss.h | 1 + 2 files changed, 108 insertions(+), 34 deletions(-) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 2aa0c2acd059..86bfd253e295 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -375,7 +375,7 @@ static int am65_cpsw_nuss_rx_push(struct am65_cpsw_common *common, cppi5_hdesc_init(desc_rx, CPPI5_INFO0_HDESC_EPIB_PRESENT, AM65_CPSW_NAV_PS_DATA_SIZE); - cppi5_hdesc_attach_buf(desc_rx, 0, 0, buf_dma, skb_tailroom(skb)); + cppi5_hdesc_attach_buf(desc_rx, buf_dma, skb_tailroom(skb), buf_dma, skb_tailroom(skb)); swdata = cppi5_hdesc_get_swdata(desc_rx); *((void **)swdata) = skb; @@ -911,10 +911,57 @@ static void am65_cpsw_nuss_tx_cleanup(void *data, dma_addr_t desc_dma) dev_kfree_skb_any(skb); } +static struct sk_buff * +am65_cpsw_nuss_tx_compl_packet(struct am65_cpsw_tx_chn *tx_chn, + dma_addr_t desc_dma) +{ + struct am65_cpsw_ndev_priv *ndev_priv; + struct am65_cpsw_ndev_stats *stats; + struct cppi5_host_desc_t *desc_tx; + struct net_device *ndev; + struct sk_buff *skb; + void **swdata; + + desc_tx = k3_cppi_desc_pool_dma2virt(tx_chn->desc_pool, +desc_dma); + swdata = cppi5_hdesc_get_swdata(desc_tx); + skb = *(swdata); + am65_cpsw_nuss_xmit_free(tx_chn, tx_chn->common->dev, desc_tx); + + ndev = skb->dev; + + am65_cpts_tx_timestamp(tx_chn->common->cpts, skb); + + ndev_priv = netdev_priv(ndev); + stats = this_cpu_ptr(ndev_priv->stats); + u64_stats_update_begin(&stats->syncp); + stats->tx_packets++; + stats->tx_bytes += skb->len; + u64_stats_update_end(&stats->syncp); + + return skb; +} + +static void am65_cpsw_nuss_tx_wake(struct am65_cpsw_tx_chn *tx_chn, struct net_device *ndev, + struct netdev_queue *netif_txq) +{ + if (netif_tx_queue_stopped(netif_txq)) { + /* Check whether the queue is stopped due to stalled +* tx dma, if the queue is stopped then wake the queue +* as we have free desc for tx +*/ + __netif_tx_lock(netif_txq, smp_processor_id()); + if (netif_running(ndev) && + (k3_cppi_desc_pool_avail(tx_chn->desc_pool) >= MAX_SKB_FRAGS)) + netif_tx_wake_queue(netif_txq); + + __netif_tx_unlock(netif_txq); + } +} + static int am65_cpsw_nuss_tx_compl_packets(struct am65_cpsw_common *common, int chn, unsigned int budget) { - struct cppi5_host_desc_t *desc_tx; struct device *dev = common->dev; struct am65_cpsw_tx_chn *tx_chn; struct netdev_queue *netif_txq; @@ -923,15 +970,13 @@ static int am65_cpsw_nuss_tx_compl_packets(struct am65_cpsw_common *common, struct sk_buff *skb; dma_addr_t desc_dma; int res, num_tx = 0; - void **swdata; tx_chn = &common->tx_chns[chn]; while (true) { - struct am65_cpsw_ndev_priv *ndev_priv; - struct am65_cpsw_ndev_stats *stats; - + spin_lock(&tx_chn->lock); res = k3_udma_glue_pop_tx_chn(tx_chn->tx_chn, &desc_dma); + spin_unlock(&tx_chn->lock); if (res == -ENODATA) break; @@ -941,23 +986,52 @@ static int am65_cpsw_nuss_tx_compl_packets(struct am65_cpsw_common *common, break; } - desc_tx = k3_cppi_desc_pool_dma2virt(tx_chn->desc_pool, -desc_dma); - swdata = cppi5_hdesc
[PATCH net-next v2 6/9] net: ethernet: ti: am65-cpsw: keep active if cpts enabled
Some K3 CPSW NUSS instances can lose context after PM runtime ON->OFF->ON transition depending on integration (including all submodules: CPTS, MDIO, etc), like J721E Main CPSW (CPSW9G). In case CPTS is enabled it's initialized during probe and does not expect to be reset. Hence, keep K3 CPSW active by forbidding PM runtime if CPTS is enabled. Signed-off-by: Grygorii Strashko --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index fecaf6b8270f..0bc0eec46709 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -1727,6 +1727,13 @@ static int am65_cpsw_init_cpts(struct am65_cpsw_common *common) return ret; } common->cpts = cpts; + /* Forbid PM runtime if CPTS is running. +* K3 CPSWxG modules may completely lose context during ON->OFF +* transitions depending on integration. +* AM65x/J721E MCU CPSW2G: false +* J721E MAIN_CPSW9G: true +*/ + pm_runtime_forbid(dev); return 0; } -- 2.17.1
[PATCH net-next v2 7/9] net: ethernet: ti: am65-cpsw: fix tx csum offload for multi mac mode
The current implementation uses .ndo_set_features() callback to track NETIF_F_HW_CSUM feature changes and update generic CPSW_P0_CONTROL_REG.RX_CHECKSUM_EN option accordingly. It's not going to work in case of multi-port devices as TX csum offload can be changed per netdev. On K3 CPSWxG devices TX csum offload enabled in the following way: - the CPSW_P0_CONTROL_REG.RX_CHECKSUM_EN option enables TX csum offload in generic and affects all TX DMA channels and packets; - corresponding fields in TX DMA descriptor have to be filed properly when upper layer wants to offload TX csum (skb->ip_summed == CHECKSUM_PARTIAL) and it's per-packet option. The Linux Network core is expected to never request TX csum offload if netdev NETIF_F_HW_CSUM feature is disabled, and, as result, TX DMA descriptors should not be modified, and per-packet TX csum offload will be disabled (or enabled) on per-netdev basis. Which, in turn, makes it safe to enable the CPSW_P0_CONTROL_REG.RX_CHECKSUM_EN option unconditionally. Hence, fix TX csum offload for multi-port devices by: - enabling the CPSW_P0_CONTROL_REG.RX_CHECKSUM_EN option in am65_cpsw_nuss_common_open() unconditionally - and removing .ndo_set_features() callback implementation, which was used only NETIF_F_HW_CSUM feature update purposes Signed-off-by: Grygorii Strashko --- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 30 +--- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 0bc0eec46709..2aa0c2acd059 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -428,9 +428,7 @@ static int am65_cpsw_nuss_common_open(struct am65_cpsw_common *common, writel(common->rx_flow_id_base, host_p->port_base + AM65_CPSW_PORT0_REG_FLOW_ID_OFFSET); /* en tx crc offload */ - if (features & NETIF_F_HW_CSUM) - writel(AM65_CPSW_P0_REG_CTL_RX_CHECKSUM_EN, - host_p->port_base + AM65_CPSW_P0_REG_CTL); + writel(AM65_CPSW_P0_REG_CTL_RX_CHECKSUM_EN, host_p->port_base + AM65_CPSW_P0_REG_CTL); am65_cpsw_nuss_set_p0_ptype(common); @@ -1371,31 +1369,6 @@ static void am65_cpsw_nuss_ndo_get_stats(struct net_device *dev, stats->tx_dropped = dev->stats.tx_dropped; } -static int am65_cpsw_nuss_ndo_slave_set_features(struct net_device *ndev, -netdev_features_t features) -{ - struct am65_cpsw_common *common = am65_ndev_to_common(ndev); - netdev_features_t changes = features ^ ndev->features; - struct am65_cpsw_host *host_p; - - host_p = am65_common_get_host(common); - - if (changes & NETIF_F_HW_CSUM) { - bool enable = !!(features & NETIF_F_HW_CSUM); - - dev_info(common->dev, "Turn %s tx-checksum-ip-generic\n", -enable ? "ON" : "OFF"); - if (enable) - writel(AM65_CPSW_P0_REG_CTL_RX_CHECKSUM_EN, - host_p->port_base + AM65_CPSW_P0_REG_CTL); - else - writel(0, - host_p->port_base + AM65_CPSW_P0_REG_CTL); - } - - return 0; -} - static const struct net_device_ops am65_cpsw_nuss_netdev_ops_2g = { .ndo_open = am65_cpsw_nuss_ndo_slave_open, .ndo_stop = am65_cpsw_nuss_ndo_slave_stop, @@ -1408,7 +1381,6 @@ static const struct net_device_ops am65_cpsw_nuss_netdev_ops_2g = { .ndo_vlan_rx_add_vid= am65_cpsw_nuss_ndo_slave_add_vid, .ndo_vlan_rx_kill_vid = am65_cpsw_nuss_ndo_slave_kill_vid, .ndo_do_ioctl = am65_cpsw_nuss_ndo_slave_ioctl, - .ndo_set_features = am65_cpsw_nuss_ndo_slave_set_features, .ndo_setup_tc = am65_cpsw_qos_ndo_setup_tc, }; -- 2.17.1
[PATCH net-next v2 9/9] net: ethernet: ti: am65-cpsw: add multi port support in mac-only mode
This patch adds final multi-port support to TI AM65x CPSW driver path in preparation for adding support for multi-port devices, like Main CPSW0 on K3 J721E SoC or future CPSW3g on K3 AM64x SoC. - the separate netdev is created for every enabled external Port; - DMA channels are common/shared for all external Ports and the RX/TX NAPI and DMA processing assigned to first available netdev; - external Ports are configured in mac-only mode, which is similar to TI "dual-mac" mode for legacy TI CPSW - packets are sent to the Host port only in ingress and directly to the Port on egress. No packet switching between external ports happens. - every port supports the same features as current AM65x CPSW on external device. Signed-off-by: Grygorii Strashko --- changes in v2: - fixed the case when Port 1 is disabled. The first *available* netdev is used to assign RX/TX NAPI and DMA processing drivers/net/ethernet/ti/am65-cpsw-nuss.c | 129 ++- drivers/net/ethernet/ti/am65-cpsw-nuss.h | 1 + 2 files changed, 82 insertions(+), 48 deletions(-) diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c index 86bfd253e295..feb94b813ffc 100644 --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c @@ -272,8 +272,8 @@ static int am65_cpsw_nuss_ndo_slave_kill_vid(struct net_device *ndev, return ret; } -static void am65_cpsw_slave_set_promisc_2g(struct am65_cpsw_port *port, - bool promisc) +static void am65_cpsw_slave_set_promisc(struct am65_cpsw_port *port, + bool promisc) { struct am65_cpsw_common *common = port->common; @@ -298,7 +298,7 @@ static void am65_cpsw_nuss_ndo_slave_set_rx_mode(struct net_device *ndev) bool promisc; promisc = !!(ndev->flags & IFF_PROMISC); - am65_cpsw_slave_set_promisc_2g(port, promisc); + am65_cpsw_slave_set_promisc(port, promisc); if (promisc) return; @@ -629,13 +629,13 @@ static int am65_cpsw_nuss_ndo_slave_open(struct net_device *ndev) am65_cpsw_port_set_sl_mac(port, ndev->dev_addr); - if (port->slave.mac_only) + if (port->slave.mac_only) { /* enable mac-only mode on port */ cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_MACONLY, 1); - if (AM65_CPSW_IS_CPSW2G(common)) cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_NOLEARN, 1); + } port_mask = BIT(port->port_id) | ALE_PORT_HOST; cpsw_ale_add_ucast(common->ale, ndev->dev_addr, @@ -1441,7 +1441,7 @@ static void am65_cpsw_nuss_ndo_get_stats(struct net_device *dev, stats->tx_dropped = dev->stats.tx_dropped; } -static const struct net_device_ops am65_cpsw_nuss_netdev_ops_2g = { +static const struct net_device_ops am65_cpsw_nuss_netdev_ops = { .ndo_open = am65_cpsw_nuss_ndo_slave_open, .ndo_stop = am65_cpsw_nuss_ndo_slave_stop, .ndo_start_xmit = am65_cpsw_nuss_ndo_slave_xmit, @@ -1463,7 +1463,6 @@ static void am65_cpsw_nuss_slave_disable_unused(struct am65_cpsw_port *port) if (!port->disabled) return; - common->disabled_ports_mask |= BIT(port->port_id); cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_DISABLE); @@ -1832,8 +1831,10 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common) return PTR_ERR(port->slave.mac_sl); port->disabled = !of_device_is_available(port_np); - if (port->disabled) + if (port->disabled) { + common->disabled_ports_mask |= BIT(port->port_id); continue; + } port->slave.ifphy = devm_of_phy_get(dev, port_np, NULL); if (IS_ERR(port->slave.ifphy)) { @@ -1887,6 +1888,12 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common) } of_node_put(node); + /* is there at least one ext.port */ + if (!(~common->disabled_ports_mask & GENMASK(common->port_num, 1))) { + dev_err(dev, "No Ext. port are available\n"); + return -ENODEV; + } + return 0; } @@ -1897,14 +1904,18 @@ static void am65_cpsw_pcpu_stats_free(void *data) free_percpu(stats); } -static int am65_cpsw_nuss_init_ndev_2g(struct am65_cpsw_common *common) +static int +am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx) { struct am65_cpsw_ndev_priv *ndev_priv; struct device *dev = common->dev; struct am65_cpsw_port *port; int ret; - port = am65_common_get_port(common, 1); + port = &common->p
Re: 5.10-rc0: build error in ipi.c
On Thu, Oct 15 2020 at 20:41, Marc Zyngier wrote: > On 2020-10-15 18:18, Pavel Machek wrote: > diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig > index 10a5aff4eecc..db923e0da162 100644 > --- a/kernel/irq/Kconfig > +++ b/kernel/irq/Kconfig > @@ -81,6 +81,7 @@ config IRQ_FASTEOI_HIERARCHY_HANDLERS > > # Generic IRQ IPI support > config GENERIC_IRQ_IPI > + select IRQ_DOMAIN_HIERARCHY > bool which makes some of the MIPS GENERIC_IRQ_IPI/IRQ_DOMAIN_HIERARCHY Kconfig magic in drivers/irqchip/Kconfig obsolete. Thanks, tglx
Re: [PATCH v5] net: can: Introduce MEN 16Z192-00 CAN controller driver
Hi Abhijeet, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on 549738f15da0e5a00275977623be199fbbf7df50] url: https://github.com/0day-ci/linux/commits/Abhijeet-Badurkar/net-can-Introduce-MEN-16Z192-00-CAN-controller-driver/20201005-192132 base:549738f15da0e5a00275977623be199fbbf7df50 config: openrisc-randconfig-s031-20201015 (attached as .config) compiler: or1k-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-rc1-dirty # https://github.com/0day-ci/linux/commit/267876771a434b2be3278c2c87d36146c0fac77d git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Abhijeet-Badurkar/net-can-Introduce-MEN-16Z192-00-CAN-controller-driver/20201005-192132 git checkout 267876771a434b2be3278c2c87d36146c0fac77d # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=openrisc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot "sparse warnings: (new ones prefixed by >>)" >> drivers/net/can/men_z192_can.c:1057:17: sparse: sparse: incorrect type in >> argument 1 (different address spaces) @@ expected void *addr @@ got >> void [noderef] __iomem *[assigned] dev_base @@ >> drivers/net/can/men_z192_can.c:1057:17: sparse: expected void *addr >> drivers/net/can/men_z192_can.c:1057:17: sparse: got void [noderef] >> __iomem *[assigned] dev_base >> drivers/net/can/men_z192_can.c:1071:21: sparse: sparse: incorrect type in >> argument 1 (different address spaces) @@ expected void *addr @@ got >> void [noderef] __iomem *dev_base @@ drivers/net/can/men_z192_can.c:1071:21: sparse: expected void *addr >> drivers/net/can/men_z192_can.c:1071:21: sparse: got void [noderef] >> __iomem *dev_base vim +1057 drivers/net/can/men_z192_can.c 962 963 static int men_z192_probe(struct mcb_device *mdev, 964const struct mcb_device_id *id) 965 { 966 struct device *dev = &mdev->dev; 967 struct men_z192 *priv; 968 struct net_device *ndev; 969 void __iomem *dev_base; 970 struct resource *mem; 971 u32 timebase; 972 int ret = 0; 973 int irq; 974 975 mem = mcb_request_mem(mdev, dev_name(dev)); 976 if (IS_ERR(mem)) { 977 dev_err(dev, "failed to request device memory"); 978 return PTR_ERR(mem); 979 } 980 981 dev_base = ioremap(mem->start, resource_size(mem)); 982 if (!dev_base) { 983 dev_err(dev, "failed to ioremap device memory"); 984 ret = -ENXIO; 985 goto out_release; 986 } 987 988 irq = mcb_get_irq(mdev); 989 if (irq <= 0) { 990 ret = -ENODEV; 991 goto out_unmap; 992 } 993 994 ndev = alloc_candev(sizeof(struct men_z192), 0); 995 if (!ndev) { 996 dev_err(dev, "failed to allocat the can device"); 997 ret = -ENOMEM; 998 goto out_unmap; 999 } 1000 1001 ndev->netdev_ops = &men_z192_netdev_ops; 1002 ndev->irq = irq; 1003 ndev->flags |= IFF_ECHO; 1004 1005 priv = netdev_priv(ndev); 1006 priv->ndev = ndev; 1007 priv->dev = dev; 1008 1009 priv->mem = mem; 1010 priv->dev_base = dev_base; 1011 priv->regs = priv->dev_base + MEN_Z192_REGS_OFFS; 1012 1013 timebase = readl(&priv->regs->timebase); 1014 if (!timebase) { 1015 dev_err(dev, "invalid timebase configured (timebase=%d)\n", 1016 timebase); 1017 ret = -EINVAL; 1018 goto out_free_candev; 1019 } 1020 1021 priv->can.clock.freq = timebase; 1022 priv->can.bittiming_const = &men_z192_bittiming_const; 1023 priv->can.do_set_mode = men_z192_set_mode; 1024 priv->can.do_get_berr_counter = men_z192_get_berr_counter; 1025 priv->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY | 1026 CAN_CTRLMODE_3_SAMPLES | 1027 CAN_CTRLMODE_LOOPBACK; 1028 1029
Re: [PATCH v7 4/4] arm64: dts: Add Caninos Loucos Labrador v3
Em 9/22/20 7:26 AM, Catalin Marinas escreveu: On Tue, Sep 22, 2020 at 10:32:06AM +0200, Arnd Bergmann wrote: On Tue, Sep 22, 2020 at 8:15 AM Manivannan Sadhasivam wrote: On Mon, Sep 21, 2020 at 11:43:02PM -0300, Matheus Castello wrote: + /* Labrador v3 firmware does not support PSCI */ Oops. This is unfortunate... I'm not sure if this is even acceptable for ARM64 machines. Let me add Olof and Arnd... Adding Catalin and Will for additional input as well, this is more their area than ours. I don't think we have formalized this as a policy, but we clearly don't want new boards to use the spin table hack. As there are other boards using psci on the same chip, I don't think this is a hardware bug. I fully agree, we shouldn't allow new boards to use the spin-table method unless EL3 is missing on the CPU implementation (not the case here; only the APM hardware has this issue). Unfortunately we missed another platform with A53, see commit bc66392d8258 ("arm64: dts: fsl: Add device tree for S32V234-EVB"). The kernel relies on firmware for other things (power management, errata workarounds), so an SMC calling convention compliant firmware is highly recommended. I also don't see why it would be that hard to add PSCI. Even if you don't port something like Trusted Firmware, U-Boot has basic support for PSCI. So from my perspective, NAK on this patch. Thanks Arnd and Catalin, this is really just a limitation of the bootloader developed by manufactures that comes embedded in the board. I will drop this in the next version. I'm tempted to also modify smp_spin_table_cpu_init() to print a big warning and return an error if this is attempted on new platforms. IOW, we make it a policy from now on.
Re: [PATCH net-next v2 0/9] net: ethernet: ti: am65-cpsw: add multi port support in mac-only mode
On 16/10/2020 02:19, Grygorii Strashko wrote: Hi This series adds multi-port support in mac-only mode (multi MAC mode) to TI AM65x CPSW driver in preparation for enabling support for multi-port devices, like Main CPSW0 on K3 J721E SoC or future CPSW3g on K3 AM64x SoC. The multi MAC mode is implemented by configuring every enabled port in "mac-only" mode (all ingress packets are sent only to the Host port and egress packets directed to target Ext. Port) and creating separate net_device for every enabled Ext. port. This series does not affect on existing CPSW2g one Ext. Port devices and xmit path changes are done only for multi-port devices by splitting xmit path for one-port and multi-port devices. Patches 1-3: Preparation patches to improve K3 CPSW configuration depending on DT Patches 4-5: Fix VLAN offload for multi MAC mode Patch 6: Fixes CPTS context lose issue during PM runtime transition Patch 7: Fixes TX csum offload for multi MAC mode Patches 8-9: add multi-port support to TI AM65x CPSW changes in v2: - patch 8: xmit path split for one-port and multi-port devices to avoid performance losses - patch 9: fixed the case when Port 1 is disabled - Patch 7: added fix for TX csum offload v1: https://lore.kernel.org/patchwork/cover/1315766/ Grygorii Strashko (9): net: ethernet: ti: am65-cpsw: move ale selection in pdata net: ethernet: ti: am65-cpsw: move free desc queue mode selection in pdata net: ethernet: ti: am65-cpsw: use cppi5_desc_is_tdcm() net: ethernet: ti: cpsw_ale: add cpsw_ale_vlan_del_modify() net: ethernet: ti: am65-cpsw: fix vlan offload for multi mac mode net: ethernet: ti: am65-cpsw: keep active if cpts enabled net: ethernet: ti: am65-cpsw: fix tx csum offload for multi mac mode net: ethernet: ti: am65-cpsw: prepare xmit/rx path for multi-port devices in mac-only mode net: ethernet: ti: am65-cpsw: add multi port support in mac-only mode drivers/net/ethernet/ti/am65-cpsw-nuss.c | 327 +++ drivers/net/ethernet/ti/am65-cpsw-nuss.h | 5 + drivers/net/ethernet/ti/cpsw_ale.c | 41 ++- drivers/net/ethernet/ti/cpsw_ale.h | 1 + drivers/net/ethernet/ti/cpsw_switchdev.c | 2 +- 5 files changed, 251 insertions(+), 125 deletions(-) Sorry, missed "net-next is CLOSED" announcement -- Best regards, grygorii
Re: [GIT PULL] SafeSetID changes for v5.10
On Thu, Oct 15, 2020 at 4:06 PM Linus Torvalds wrote: > > These were rebased since the merge window started, for no apparent reason. > > Were they in linux-next? Yeah, they are changes that were originally targeting the v5.9 merge window (and thus were in -next during July/August) but I didn't get the chance to send a pull request for them. Since I didn't touch my -next branch since then they are also in 'next-20201013' and 'next-20201015'. I just rebased to v5.9 to make sure the 1-line changes that touch kernel/capability.c, kernel/groups.c and kernel/sys.c still applied cleanly without conflicts. Should I have rebased onto one of the -rc's for v5.9 instead? > > And if so, why was I sent some different version? > > Linus
linux-next: manual merge of the djw-vfs tree with Linus' tree
Hi all, Today's linux-next merge of the djw-vfs tree got a conflict in: fs/Makefile between commit: 5287b07f6d7c ("fs/kernel_read_file: Split into separate source file") from Linus' tree and commit: 02e83f46ebfa ("vfs: move generic_remap_checks out of mm") from the djw-vfs tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc fs/Makefile index 7bb2a05fda1f,7173350739c5.. --- a/fs/Makefile +++ b/fs/Makefile @@@ -14,7 -14,7 +14,7 @@@ obj-y := open.o read_write.o file_table pnode.o splice.o sync.o utimes.o d_path.o \ stack.o fs_struct.o statfs.o fs_pin.o nsfs.o \ fs_types.o fs_context.o fs_parser.o fsopen.o init.o \ - kernel_read_file.o - remap_range.o ++ kernel_read_file.o remap_range.o ifeq ($(CONFIG_BLOCK),y) obj-y += buffer.o block_dev.o direct-io.o mpage.o pgpq8wqTbXDGO.pgp Description: OpenPGP digital signature
[PATCH v9 01/15] PCI/RCEC: Add RCEC class code and extended capability
From: Qiuxu Zhuo A PCIe Root Complex Event Collector (RCEC) has base class 0x08, sub-class 0x07, and programming interface 0x00. Add the class code 0x0807 to identify RCEC devices and add #defines for the RCEC Endpoint Association Extended Capability. See PCIe r5.0, sec 1.3.4 ("Root Complex Event Collector") and sec 7.9.10 ("Root Complex Event Collector Endpoint Association Extended Capability"). Link: https://lore.kernel.org/r/20201002184735.1229220-2-seanvk@oregontracks.org Signed-off-by: Qiuxu Zhuo Signed-off-by: Bjorn Helgaas Reviewed-by: Jonathan Cameron --- include/linux/pci_ids.h | 1 + include/uapi/linux/pci_regs.h | 7 +++ 2 files changed, 8 insertions(+) diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h index 1ab1e24bcbce..d8156a5dbee8 100644 --- a/include/linux/pci_ids.h +++ b/include/linux/pci_ids.h @@ -81,6 +81,7 @@ #define PCI_CLASS_SYSTEM_RTC 0x0803 #define PCI_CLASS_SYSTEM_PCI_HOTPLUG 0x0804 #define PCI_CLASS_SYSTEM_SDHCI 0x0805 +#define PCI_CLASS_SYSTEM_RCEC 0x0807 #define PCI_CLASS_SYSTEM_OTHER 0x0880 #define PCI_BASE_CLASS_INPUT 0x09 diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index f9701410d3b5..f6475a9e63d8 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -828,6 +828,13 @@ #define PCI_PWR_CAP_BUDGET(x) ((x) & 1) /* Included in system budget */ #define PCI_EXT_CAP_PWR_SIZEOF 16 +/* Root Complex Event Collector Endpoint Association */ +#define PCI_RCEC_RCIEP_BITMAP 4 /* Associated Bitmap for RCiEPs */ +#define PCI_RCEC_BUSN 8 /* RCEC Associated Bus Numbers */ +#define PCI_RCEC_BUSN_REG_VER 0x02/* Least version with BUSN present */ +#define PCI_RCEC_BUSN_NEXT(x) (((x) >> 8) & 0xff) +#define PCI_RCEC_BUSN_LAST(x) (((x) >> 16) & 0xff) + /* Vendor-Specific (VSEC, PCI_EXT_CAP_ID_VNDR) */ #define PCI_VNDR_HEADER4 /* Vendor-Specific Header */ #define PCI_VNDR_HEADER_ID(x) ((x) & 0x) -- 2.28.0
[PATCH v9 00/15] Add RCEC handling to PCI/AER
From: Sean V Kelley Changes since v8 [1] and based on discussion [2] and pci/err tree [3]: - No functional changes. Tested with aer injection. PCI/AER: Apply function level reset to RCiEP on fatal error - Remove. Handle with pcie_flr() directly when adding linked RCEC to AER/ERR. PCI/RCEC: Add RCiEP's linked RCEC to AER/ERR - Just call pcie_flr() and remove need for wrapping with flr_on_rciep(). Note it appears that a check on pcie_has_flr() (as also used in flr_on_rciep())relates to hardware specific quirks and so I've added it. - Consolidate AER register setting in aer_root_reset() with a test for the non-native case. With that change, simplify "state == pci_channel_io_frozen" case by removing tests for the non-native case. Also simplify pci_walk_bridge(). (Bjorn Helgaas) [1] https://lore.kernel.org/lkml/20201002184735.1229220-1-seanvk@oregontracks.org/ [2] https://lore.kernel.org/lkml/20201009213011.GA3504871@bjorn-Precision-5520/ [3] https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/err Root Complex Event Collectors (RCEC) provide support for terminating error and PME messages from Root Complex Integrated Endpoints (RCiEPs). An RCEC resides on a Bus in the Root Complex. Multiple RCECs can in fact reside on a single bus. An RCEC will explicitly declare supported RCiEPs through the Root Complex Endpoint Association Extended Capability. (See PCIe 5.0-1, sections 1.3.2.3 (RCiEP), and 7.9.10 (RCEC Ext. Cap.)) The kernel lacks handling for these RCECs and the error messages received from their respective associated RCiEPs. More recently, a new CPU interconnect, Compute eXpress Link (CXL) depends on RCEC capabilities for purposes of error messaging from CXL 1.1 supported RCiEP devices. DocLink: https://www.computeexpresslink.org/ This use case is not limited to CXL. Existing hardware today includes support for RCECs, such as the Denverton microserver product family. Future hardware will be forthcoming. (See Intel Document, Order number: 33061-003US) So services such as AER or PME could be associated with an RCEC driver. In the case of CXL, if an RCiEP (i.e., CXL 1.1 device) is associated with a platform's RCEC it shall signal PME and AER error conditions through that RCEC. Towards the above use cases, add the missing RCEC class and extend the PCIe Root Port and service drivers to allow association of RCiEPs to their respective parent RCEC and facilitate handling of terminating error and PME messages. Tested-by: Jonathan Cameron #non-native/no RCEC Qiuxu Zhuo (4): PCI/RCEC: Add RCEC class code and extended capability PCI/RCEC: Bind RCEC devices to the Root Port driver PCI/RCEC: Add RCiEP's linked RCEC to AER/ERR PCI/AER: Add RCEC AER error injection support Sean V Kelley (11): PCI/RCEC: Cache RCEC capabilities in pci_init_capabilities() PCI/ERR: Rename reset_link() to reset_subordinates() PCI/ERR: Simplify by using pci_upstream_bridge() PCI/ERR: Simplify by computing pci_pcie_type() once PCI/ERR: Use "bridge" for clarity in pcie_do_recovery() PCI/ERR: Avoid negated conditional for clarity PCI/ERR: Add pci_walk_bridge() to pcie_do_recovery() PCI/ERR: Limit AER resets in pcie_do_recovery() PCI/RCEC: Add pcie_link_rcec() to associate RCiEPs PCI/AER: Add pcie_walk_rcec() to RCEC AER handling PCI/PME: Add pcie_walk_rcec() to RCEC PME handling drivers/pci/pci.h | 29 - drivers/pci/pcie/Makefile | 2 +- drivers/pci/pcie/aer.c | 82 ++ drivers/pci/pcie/aer_inject.c | 5 +- drivers/pci/pcie/err.c | 93 +++- drivers/pci/pcie/pme.c | 15 ++- drivers/pci/pcie/portdrv_core.c | 9 +- drivers/pci/pcie/portdrv_pci.c | 8 +- drivers/pci/pcie/rcec.c | 190 drivers/pci/probe.c | 2 + include/linux/pci.h | 5 + include/linux/pci_ids.h | 1 + include/uapi/linux/pci_regs.h | 7 ++ 13 files changed, 384 insertions(+), 64 deletions(-) create mode 100644 drivers/pci/pcie/rcec.c -- 2.28.0
[PATCH v9 14/15] PCI/PME: Add pcie_walk_rcec() to RCEC PME handling
From: Sean V Kelley Root Complex Event Collectors (RCEC) appear as peers of Root Ports and also have the PME capability. As with AER, there is a need to be able to walk the RCiEPs associated with their RCEC for purposes of acting upon them with callbacks. Add RCEC support through the use of pcie_walk_rcec() to the current PME service driver and attach the PME service driver to the RCEC device. Co-developed-by: Qiuxu Zhuo Link: https://lore.kernel.org/r/20201002184735.1229220-14-seanvk@oregontracks.org Signed-off-by: Qiuxu Zhuo Signed-off-by: Sean V Kelley Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/pme.c | 15 +++ drivers/pci/pcie/portdrv_core.c | 9 +++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/pci/pcie/pme.c b/drivers/pci/pcie/pme.c index 6a32970bb731..87799166c96a 100644 --- a/drivers/pci/pcie/pme.c +++ b/drivers/pci/pcie/pme.c @@ -310,7 +310,10 @@ static int pcie_pme_can_wakeup(struct pci_dev *dev, void *ign) static void pcie_pme_mark_devices(struct pci_dev *port) { pcie_pme_can_wakeup(port, NULL); - if (port->subordinate) + + if (pci_pcie_type(port) == PCI_EXP_TYPE_RC_EC) + pcie_walk_rcec(port, pcie_pme_can_wakeup, NULL); + else if (port->subordinate) pci_walk_bus(port->subordinate, pcie_pme_can_wakeup, NULL); } @@ -320,10 +323,15 @@ static void pcie_pme_mark_devices(struct pci_dev *port) */ static int pcie_pme_probe(struct pcie_device *srv) { - struct pci_dev *port; + struct pci_dev *port = srv->port; struct pcie_pme_service_data *data; int ret; + /* Limit to Root Ports or Root Complex Event Collectors */ + if ((pci_pcie_type(port) != PCI_EXP_TYPE_RC_EC) && + (pci_pcie_type(port) != PCI_EXP_TYPE_ROOT_PORT)) + return -ENODEV; + data = kzalloc(sizeof(*data), GFP_KERNEL); if (!data) return -ENOMEM; @@ -333,7 +341,6 @@ static int pcie_pme_probe(struct pcie_device *srv) data->srv = srv; set_service_data(srv, data); - port = srv->port; pcie_pme_interrupt_enable(port, false); pcie_clear_root_pme_status(port); @@ -445,7 +452,7 @@ static void pcie_pme_remove(struct pcie_device *srv) static struct pcie_port_service_driver pcie_pme_driver = { .name = "pcie_pme", - .port_type = PCI_EXP_TYPE_ROOT_PORT, + .port_type = PCIE_ANY_PORT, .service= PCIE_PORT_SERVICE_PME, .probe = pcie_pme_probe, diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c index 50a9522ab07d..e1fed6649c41 100644 --- a/drivers/pci/pcie/portdrv_core.c +++ b/drivers/pci/pcie/portdrv_core.c @@ -233,12 +233,9 @@ static int get_port_device_capability(struct pci_dev *dev) } #endif - /* -* Root ports are capable of generating PME too. Root Complex -* Event Collectors can also generate PMEs, but we don't handle -* those yet. -*/ - if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT && + /* Root Ports and Root Complex Event Collectors may generate PMEs */ + if ((pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT || +pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC) && (pcie_ports_native || host->native_pme)) { services |= PCIE_PORT_SERVICE_PME; -- 2.28.0