* Re: request to add pfr-manager repo to CI
From: Andrew Geissler @ 2020-05-29 19:51 UTC (permalink / raw)
To: Puli, Apparao; +Cc: vikram.bodireddy, OpenBMC Maillist
In-Reply-To: <63ac8cfd-86b9-43fe-91a0-60f60b4469ee@linux.intel.com>
> On May 29, 2020, at 10:33 AM, Puli, Apparao <apparao.puli@linux.intel.com> wrote:
>
> Hi Andrew,
>
> Can you please add "pfr-manager" repo to CI builds?
Hey Apparao, sure, done.
>
> https://github.com/openbmc/pfr-manager
>
> https://gerrit.openbmc-project.xyz/#/q/project:openbmc/pfr-manager
>
>
> Thanks,
>
> -Appu
>
^ permalink raw reply
* [PATCH] spi: spi-fsl-dspi: fix native data copy
From: Angelo Dureghello @ 2020-05-29 19:57 UTC (permalink / raw)
To: broonie; +Cc: linux-spi, vladimir.oltean, Angelo Dureghello
ColdFire is a big-endian cpu with a big-endian dspi hw module,
so, it uses native access, but memcpy breaks the endianness.
So, if i understand properly, by native copy we would mean
be(cpu)->be(dspi) or le(cpu)->le(dspi) accesses, so my fix
shouldn't break anything, but i couldn't test it on LS family,
so every test is really appreciated.
Signed-off-by: Angelo Dureghello <angelo.dureghello@timesys.com>
---
drivers/spi/spi-fsl-dspi.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 50e41f66a2d7..2e9f9adc5900 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -246,13 +246,33 @@ struct fsl_dspi {
static void dspi_native_host_to_dev(struct fsl_dspi *dspi, u32 *txdata)
{
- memcpy(txdata, dspi->tx, dspi->oper_word_size);
+ switch (dspi->oper_word_size) {
+ case 1:
+ *txdata = *(u8 *)dspi->tx;
+ break;
+ case 2:
+ *txdata = *(u16 *)dspi->tx;
+ break;
+ case 4:
+ *txdata = *(u32 *)dspi->tx;
+ break;
+ }
dspi->tx += dspi->oper_word_size;
}
static void dspi_native_dev_to_host(struct fsl_dspi *dspi, u32 rxdata)
{
- memcpy(dspi->rx, &rxdata, dspi->oper_word_size);
+ switch (dspi->oper_word_size) {
+ case 1:
+ *(u8 *)dspi->rx = rxdata;
+ break;
+ case 2:
+ *(u16 *)dspi->rx = rxdata;
+ break;
+ case 4:
+ *(u32 *)dspi->rx = rxdata;
+ break;
+ }
dspi->rx += dspi->oper_word_size;
}
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v2 1/2] spi: dw: Make DMA request line assignments explicit for Intel Medfield
From: Mark Brown @ 2020-05-29 19:52 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Serge Semin, linux-spi
In-Reply-To: <20200529184955.GY1634618@smile.fi.intel.com>
[-- Attachment #1: Type: text/plain, Size: 799 bytes --]
On Fri, May 29, 2020 at 09:49:55PM +0300, Andy Shevchenko wrote:
> On Fri, May 29, 2020 at 09:40:50PM +0300, Serge Semin wrote:
> > > - struct dw_dma_slave slave = {
> > > - .src_id = 0,
> > > - .dst_id = 0
> > > - };
> > > + struct dw_dma_slave dma_tx = { .dst_id = 1 }, *tx = &dma_tx;
> > > + struct dw_dma_slave dma_rx = { .src_id = 0 }, *rx = &dma_rx;
> > You know my attitude to these changes.) But anyway what's the point in having
> > the *tx and *rx pointers here? Without any harm to the readability you can use
> > the structures names directly, don't you?
> I will wait for Mark to decide.
Like I said before I don't particularly care either way, I've queued the
patch to apply but really I'd rather that the people working on the
driver could come to some sort of agreement here.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [Buildroot] [git commit] package/matio: add upstream security fixes
From: Peter Korsgaard @ 2020-05-29 19:54 UTC (permalink / raw)
To: buildroot
commit: https://git.buildroot.net/buildroot/commit/?id=e1af92592ec591270ef7f86a56562d119f2a46e1
branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master
Fix the following CVEs:
- CVE-2019-17533: Mat_VarReadNextInfo4 in mat4.c in MATIO 1.5.17 omits
a certain '\0' character, leading to a heap-based buffer over-read in
strdup_vprintf when uninitialized memory is accessed.
- CVE-2019-20017: A stack-based buffer over-read was discovered in
Mat_VarReadNextInfo5 in mat5.c in matio 1.5.17.
- CVE-2019-20018: A stack-based buffer over-read was discovered in
ReadNextCell in mat5.c in matio 1.5.17.
- CVE-2019-20020: A stack-based buffer over-read was discovered in
ReadNextStructField in mat5.c in matio 1.5.17.
- CVE-2019-20052: A memory leak was discovered in Mat_VarCalloc in
mat.c in matio 1.5.17 because SafeMulDims does not consider the
rank==0 case.
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
.../matio/0001-Avoid-uninitialized-memory.patch | 27 +++++++++++++
package/matio/0002-Fix-illegal-memory-access.patch | 47 ++++++++++++++++++++++
package/matio/0003-Fix-illegal-memory-access.patch | 46 +++++++++++++++++++++
package/matio/0004-Fix-memory-leak.patch | 39 ++++++++++++++++++
package/matio/matio.mk | 9 +++++
5 files changed, 168 insertions(+)
diff --git a/package/matio/0001-Avoid-uninitialized-memory.patch b/package/matio/0001-Avoid-uninitialized-memory.patch
new file mode 100644
index 0000000000..01fc8f0f7d
--- /dev/null
+++ b/package/matio/0001-Avoid-uninitialized-memory.patch
@@ -0,0 +1,27 @@
+From 651a8e28099edb5fbb9e4e1d4d3238848f446c9a Mon Sep 17 00:00:00 2001
+From: tbeu <tbeu@users.noreply.github.com>
+Date: Fri, 30 Aug 2019 09:21:26 +0200
+Subject: [PATCH] Avoid uninitialized memory
+
+As reported by https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=16856
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+[Retrieved from:
+https://github.com/tbeu/matio/commit/651a8e28099edb5fbb9e4e1d4d3238848f446c9a]
+---
+ src/mat4.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/src/mat4.c b/src/mat4.c
+index 601a3d6..93b4308 100644
+--- a/src/mat4.c
++++ b/src/mat4.c
+@@ -917,6 +917,8 @@ Mat_VarReadNextInfo4(mat_t *mat)
+ if ( tmp != readresult ) {
+ Mat_VarFree(matvar);
+ return NULL;
++ } else {
++ matvar->name[tmp - 1] = '\0';
+ }
+
+ matvar->internal->datapos = ftell((FILE*)mat->fp);
diff --git a/package/matio/0002-Fix-illegal-memory-access.patch b/package/matio/0002-Fix-illegal-memory-access.patch
new file mode 100644
index 0000000000..5150c79e29
--- /dev/null
+++ b/package/matio/0002-Fix-illegal-memory-access.patch
@@ -0,0 +1,47 @@
+From 7b4699854cc65874e13a8e6944cd8e62fa981068 Mon Sep 17 00:00:00 2001
+From: tbeu <tbeu@users.noreply.github.com>
+Date: Mon, 11 Nov 2019 21:58:41 +0100
+Subject: [PATCH] Fix illegal memory access
+
+As reported by https://github.com/tbeu/matio/issues/128
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+[Retrieved from:
+https://github.com/tbeu/matio/commit/7b4699854cc65874e13a8e6944cd8e62fa981068]
+---
+ src/mat5.c | 19 +++++++++++++++++--
+ 1 file changed, 17 insertions(+), 2 deletions(-)
+
+diff --git a/src/mat5.c b/src/mat5.c
+index 7f50da4..b76a331 100644
+--- a/src/mat5.c
++++ b/src/mat5.c
+@@ -1380,11 +1380,26 @@ ReadNextStructField( mat_t *mat, matvar_t *matvar )
+ /* Rank and dimension */
+ if ( uncomp_buf[0] == MAT_T_INT32 ) {
+ int j;
++ size_t size;
+ fields[i]->rank = uncomp_buf[1];
+ nbytes -= fields[i]->rank;
+ fields[i]->rank /= 4;
+- fields[i]->dims = (size_t*)malloc(fields[i]->rank*
+- sizeof(*fields[i]->dims));
++ if ( 0 == do_clean && fields[i]->rank > 13 ) {
++ int rank = fields[i]->rank;
++ fields[i]->rank = 0;
++ Mat_Critical("%d is not a valid rank", rank);
++ continue;
++ }
++ err = SafeMul(&size, fields[i]->rank, sizeof(*fields[i]->dims));
++ if ( err ) {
++ if ( do_clean )
++ free(dims);
++ Mat_VarFree(fields[i]);
++ fields[i] = NULL;
++ Mat_Critical("Integer multiplication overflow");
++ continue;
++ }
++ fields[i]->dims = (size_t*)malloc(size);
+ if ( mat->byteswap ) {
+ for ( j = 0; j < fields[i]->rank; j++ )
+ fields[i]->dims[j] = Mat_uint32Swap(dims+j);
diff --git a/package/matio/0003-Fix-illegal-memory-access.patch b/package/matio/0003-Fix-illegal-memory-access.patch
new file mode 100644
index 0000000000..787207f217
--- /dev/null
+++ b/package/matio/0003-Fix-illegal-memory-access.patch
@@ -0,0 +1,46 @@
+From 65831b7ec829b0ae0ac9d691a2f8fbc2b26af677 Mon Sep 17 00:00:00 2001
+From: tbeu <tbeu@users.noreply.github.com>
+Date: Mon, 11 Nov 2019 22:03:54 +0100
+Subject: [PATCH] Fix illegal memory access
+
+As reported by https://github.com/tbeu/matio/issues/129
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+[Retrieved from:
+https://github.com/tbeu/matio/commit/65831b7ec829b0ae0ac9d691a2f8fbc2b26af677]
+---
+ src/mat5.c | 18 +++++++++++++++++-
+ 1 file changed, 17 insertions(+), 1 deletion(-)
+
+diff --git a/src/mat5.c b/src/mat5.c
+index b76a331..5e3464e 100644
+--- a/src/mat5.c
++++ b/src/mat5.c
+@@ -989,10 +989,26 @@ ReadNextCell( mat_t *mat, matvar_t *matvar )
+ /* Rank and Dimension */
+ if ( uncomp_buf[0] == MAT_T_INT32 ) {
+ int j;
++ size_t size;
+ cells[i]->rank = uncomp_buf[1];
+ nbytes -= cells[i]->rank;
+ cells[i]->rank /= 4;
+- cells[i]->dims = (size_t*)malloc(cells[i]->rank*sizeof(*cells[i]->dims));
++ if ( 0 == do_clean && cells[i]->rank > 13 ) {
++ int rank = cells[i]->rank;
++ cells[i]->rank = 0;
++ Mat_Critical("%d is not a valid rank", rank);
++ continue;
++ }
++ err = SafeMul(&size, cells[i]->rank, sizeof(*cells[i]->dims));
++ if ( err ) {
++ if ( do_clean )
++ free(dims);
++ Mat_VarFree(cells[i]);
++ cells[i] = NULL;
++ Mat_Critical("Integer multiplication overflow");
++ continue;
++ }
++ cells[i]->dims = (size_t*)malloc(size);
+ if ( mat->byteswap ) {
+ for ( j = 0; j < cells[i]->rank; j++ )
+ cells[i]->dims[j] = Mat_uint32Swap(dims + j);
diff --git a/package/matio/0004-Fix-memory-leak.patch b/package/matio/0004-Fix-memory-leak.patch
new file mode 100644
index 0000000000..1899d995da
--- /dev/null
+++ b/package/matio/0004-Fix-memory-leak.patch
@@ -0,0 +1,39 @@
+From a47b7cd3aca70e9a0bddf8146eb4ab0cbd19c2c3 Mon Sep 17 00:00:00 2001
+From: tbeu <tbeu@users.noreply.github.com>
+Date: Fri, 15 Nov 2019 23:20:41 +0100
+Subject: [PATCH] Fix memory leak
+
+As reported by https://github.com/tbeu/matio/issues/131
+
+Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
+[Retrieved from:
+https://github.com/tbeu/matio/commit/a47b7cd3aca70e9a0bddf8146eb4ab0cbd19c2c3]
+---
+ src/mat.c | 7 ++++++-
+ 1 file changed, 6 insertions(+), 1 deletion(-)
+
+diff --git a/src/mat.c b/src/mat.c
+index c9c6bd1..e62a9d2 100644
+--- a/src/mat.c
++++ b/src/mat.c
+@@ -220,6 +220,11 @@ int SafeMulDims(const matvar_t *matvar, size_t* nelems)
+ {
+ int i;
+
++ if ( matvar->rank == 0 ) {
++ *nelems = 0;
++ return 0;
++ }
++
+ for ( i = 0; i < matvar->rank; i++ ) {
+ if ( !psnip_safe_size_mul(nelems, *nelems, matvar->dims[i]) ) {
+ *nelems = 0;
+@@ -1640,7 +1645,7 @@ Mat_VarFree(matvar_t *matvar)
+ }
+ #endif
+ if ( NULL != matvar->internal->fieldnames &&
+- matvar->internal->num_fields > 0 ) {
++ matvar->internal->num_fields > 0 ) {
+ size_t i;
+ for ( i = 0; i < matvar->internal->num_fields; i++ ) {
+ if ( NULL != matvar->internal->fieldnames[i] )
diff --git a/package/matio/matio.mk b/package/matio/matio.mk
index 8af39ce22c..b9bb476223 100644
--- a/package/matio/matio.mk
+++ b/package/matio/matio.mk
@@ -11,6 +11,15 @@ MATIO_LICENSE_FILES = COPYING
MATIO_DEPENDENCIES = zlib
MATIO_INSTALL_STAGING = YES
+# 0001-Avoid-uninitialized-memory.patch
+MATIO_IGNORE_CVES += CVE-2019-17533
+# 0002-Fix-illegal-memory-access.patch
+MATIO_IGNORE_CVES += CVE-2019-20017 CVE-2019-20020
+# 0003-Fix-illegal-memory-access.patch
+MATIO_IGNORE_CVES += CVE-2019-20017 CVE-2019-20018
+# 0004-Fix-memory-leak.patch
+MATIO_IGNORE_CVES += CVE-2019-20052
+
# va_copy()
MATIO_CONF_ENV = ac_cv_va_copy=yes
^ permalink raw reply related
* Re: [PATCH 8/8] blk-mq: drain I/O when all CPUs in a hctx are offline
From: Bart Van Assche @ 2020-05-29 19:55 UTC (permalink / raw)
To: paulmck, Ming Lei
Cc: Christoph Hellwig, linux-block, John Garry, Hannes Reinecke,
Thomas Gleixner, linux-kernel
In-Reply-To: <20200529181352.GF2869@paulmck-ThinkPad-P72>
On 2020-05-29 11:13, Paul E. McKenney wrote:
> On Fri, May 29, 2020 at 11:53:15AM +0800, Ming Lei wrote:
>> Another pair is in blk_mq_get_tag(), and we expect the following two
>> memory OPs are ordered:
>>
>> 1) set bit in successful test_and_set_bit_lock(), which is called
>> from sbitmap_get()
>>
>> 2) test_bit(BLK_MQ_S_INACTIVE, &data->hctx->state)
>>
>> Do you think that the above two OPs are ordered?
>
> Given that he has been through the code, I would like to hear Bart's
> thoughts, actually.
Hi Paul,
My understanding of the involved instructions is as follows (see also
https://lore.kernel.org/linux-block/b98f055f-6f38-a47c-965d-b6bcf4f5563f@huawei.com/T/#t
for the entire e-mail thread):
* blk_mq_hctx_notify_offline() sets the BLK_MQ_S_INACTIVE bit in
hctx->state, calls smp_mb__after_atomic() and waits in a loop until all
tags have been freed. Each tag is an integer number that has a 1:1
correspondence with a block layer request structure. The code that
iterates over block layer request tags relies on
__sbitmap_for_each_set(). That function examines both the 'word' and
'cleared' members of struct sbitmap_word.
* What blk_mq_hctx_notify_offline() waits for is freeing of tags by
blk_mq_put_tag(). blk_mq_put_tag() frees a tag by setting a bit in
sbitmap_word.cleared (see also sbitmap_deferred_clear_bit()).
* Tag allocation by blk_mq_get_tag() relies on test_and_set_bit_lock().
The actual allocation happens by sbitmap_get() that sets a bit in
sbitmap_word.word. blk_mg_get_tag() tests the BLK_MQ_S_INACTIVE bit
after tag allocation succeeded.
What confuses me is that blk_mq_hctx_notify_offline() uses
smp_mb__after_atomic() to enforce the order of memory accesses while
blk_mq_get_tag() relies on the acquire semantics of
test_and_set_bit_lock(). Usually ordering is enforced by combining two
smp_mb() calls or by combining a store-release with a load-acquire.
Does the Linux memory model provide the expected ordering guarantees
when combining load-acquire with smp_mb__after_atomic() as used in patch
8/8 of this series?
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH 24/24] nl80211/cfg80211: support 6 GHz scanning
From: kbuild test robot @ 2020-05-29 19:56 UTC (permalink / raw)
To: kbuild-all
In-Reply-To: <20200528165011.db75a9f917ca.I9d94ae093e08fb15b6c8f8fb7406b316778c6a5f@changeid>
[-- Attachment #1: Type: text/plain, Size: 6447 bytes --]
Hi Johannes,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mac80211-next/master]
[also build test WARNING on next-20200529]
[cannot apply to mac80211/master v5.7-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Johannes-Berg/nl80211-really-allow-client-only-BIGTK-support/20200528-230010
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: i386-randconfig-s002-20200529 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-243-gc100a7ab-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> net/wireless/scan.c:744:65: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cfg80211_bss_ies const *ies @@ got struct cfg80211_bss_ies const [noderef] <asn:4> *ies @@
>> net/wireless/scan.c:744:65: sparse: expected struct cfg80211_bss_ies const *ies
>> net/wireless/scan.c:744:65: sparse: got struct cfg80211_bss_ies const [noderef] <asn:4> *ies
--
>> net/wireless/nl80211.c:7996:28: sparse: sparse: too many arguments for function cfg80211_scan
vim +744 net/wireless/scan.c
720
721 static int cfg80211_scan_6ghz(struct cfg80211_registered_device *rdev)
722 {
723 u8 i;
724 struct cfg80211_colocated_ap *ap;
725 int n_channels, count = 0, err;
726 struct cfg80211_scan_request *request, *rdev_req = rdev->scan_req;
727 LIST_HEAD(coloc_ap_list);
728 bool need_scan_psc;
729
730 rdev_req->scan_6ghz = true;
731
732 if (!rdev->wiphy.bands[NL80211_BAND_6GHZ])
733 return -EOPNOTSUPP;
734
735 n_channels = rdev->wiphy.bands[NL80211_BAND_6GHZ]->n_channels;
736
737 if (rdev_req->flags & NL80211_SCAN_FLAG_COLOCATED_6GHZ) {
738 struct cfg80211_internal_bss *intbss;
739
740 spin_lock_bh(&rdev->bss_lock);
741 list_for_each_entry(intbss, &rdev->bss_list, list) {
742 struct cfg80211_bss *res = &intbss->pub;
743
> 744 count += cfg80211_parse_colocated_ap(res->ies,
745 &coloc_ap_list);
746 }
747 spin_unlock_bh(&rdev->bss_lock);
748 }
749
750 request = kzalloc(struct_size(request, channels, n_channels) +
751 sizeof(*request->scan_6ghz_params) * count,
752 GFP_KERNEL);
753 if (!request) {
754 cfg80211_free_coloc_ap_list(&coloc_ap_list);
755 return -ENOMEM;
756 }
757
758 *request = *rdev_req;
759 request->n_channels = 0;
760 request->scan_6ghz_params =
761 (void *)&request->channels[n_channels];
762
763 /*
764 * PSC channels should not be scanned if all the reported co-located APs
765 * are indicating that all APs in the same ESS are co-located
766 */
767 if (count) {
768 need_scan_psc = false;
769
770 list_for_each_entry(ap, &coloc_ap_list, list) {
771 if (!ap->colocated_ess) {
772 need_scan_psc = true;
773 break;
774 }
775 }
776 } else {
777 need_scan_psc = true;
778 }
779
780 /*
781 * add to the scan request the channels that need to be scanned
782 * regardless of the collocated APs (PSC channels or all channels
783 * in case that NL80211_SCAN_FLAG_COLOCATED_6GHZ is not set)
784 */
785 for (i = 0; i < rdev_req->n_channels; i++) {
786 if (rdev_req->channels[i]->band == NL80211_BAND_6GHZ &&
787 ((need_scan_psc &&
788 cfg80211_channel_is_psc(rdev_req->channels[i])) ||
789 !(rdev_req->flags & NL80211_SCAN_FLAG_COLOCATED_6GHZ))) {
790 cfg80211_scan_req_add_chan(request,
791 rdev_req->channels[i],
792 false);
793 }
794 }
795
796 if (!(rdev_req->flags & NL80211_SCAN_FLAG_COLOCATED_6GHZ))
797 goto skip;
798
799 list_for_each_entry(ap, &coloc_ap_list, list) {
800 bool found = false;
801 struct cfg80211_scan_6ghz_params *scan_6ghz_params =
802 &request->scan_6ghz_params[request->n_6ghz_params];
803 struct ieee80211_channel *chan =
804 ieee80211_get_channel(&rdev->wiphy, ap->center_freq);
805
806 if (!chan || chan->flags & IEEE80211_CHAN_DISABLED)
807 continue;
808
809 for (i = 0; i < rdev_req->n_channels; i++) {
810 if (rdev_req->channels[i] == chan)
811 found = true;
812 }
813
814 if (!found)
815 continue;
816
817 if (request->n_ssids > 0 &&
818 !cfg80211_find_ssid_match(ap, request))
819 continue;
820
821 cfg80211_scan_req_add_chan(request, chan, true);
822 memcpy(scan_6ghz_params->bssid, ap->bssid, ETH_ALEN);
823 scan_6ghz_params->short_ssid = ap->short_ssid;
824 scan_6ghz_params->short_ssid_valid = ap->short_ssid_valid;
825 scan_6ghz_params->unsolicited_probe = ap->unsolicited_probe;
826 request->n_6ghz_params++;
827 }
828
829 skip:
830 cfg80211_free_coloc_ap_list(&coloc_ap_list);
831
832 if (request->n_channels) {
833 struct cfg80211_scan_request *old = rdev->int_scan_req;
834
835 rdev->int_scan_req = request;
836
837 /*
838 * If this scan follows a previous scan, save the scan start
839 * info from the first part of the scan
840 */
841 if (old)
842 rdev->int_scan_req->info = old->info;
843
844 err = rdev_scan(rdev, request);
845 if (err) {
846 rdev->int_scan_req = old;
847 kfree(request);
848 } else {
849 kfree(old);
850 }
851
852 return err;
853 }
854
855 kfree(request);
856 return -EINVAL;
857 }
858
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 39899 bytes --]
^ permalink raw reply
* Re: [PATCH v2 12/14] x86/entry: Adjust guest paths to be shadow stack compatible
From: Andrew Cooper @ 2020-05-29 19:58 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel, Wei Liu, Roger Pau Monné
In-Reply-To: <5be19f55-979a-3cef-18bf-f9673cef1da3@suse.com>
On 29/05/2020 13:40, Jan Beulich wrote:
> On 27.05.2020 21:18, Andrew Cooper wrote:
>> The SYSCALL/SYSENTER/SYSRET paths need to use {SET,CLR}SSBSY. The IRET to
>> guest paths must not. In the SYSRET path, re-position the mov which loads rip
>> into %rcx so we can use %rcx for CLRSSBSY, rather than spilling another
>> register to the stack.
>>
>> While we can in principle detect shadow stack corruption and a failure to
>> clear the supervisor token busy bit in the SYSRET path (by inspecting the
>> carry flag following CLRSSBSY), we cannot detect similar problems for the IRET
>> path (IRET is specified not to fault in this case).
>>
>> We will double fault at some point later, when next trying to enter Xen, due
>> to an already-set supervisor shadow stack busy bit. As SYSRET is a uncommon
>> path anyway, avoid the added complexity for no appreciable gain.
> I'm okay with the avoidance of complexity, but why is the SYSRET path
> uncommon? Almost all hypercall returns ought to take that path?
But hypercalls returns aren't the majority of returns to guest context.
IRET from Xen IPIs, or from event channel injections hitting guest
userspace, are the most common in a non-idle system.
>
>> --- a/xen/arch/x86/x86_64/entry.S
>> +++ b/xen/arch/x86/x86_64/entry.S
>> @@ -191,9 +191,16 @@ restore_all_guest:
>> sarq $47,%rcx
>> incl %ecx
>> cmpl $1,%ecx
>> - movq 8(%rsp),%rcx # RIP
>> ja iret_exit_to_guest
> This removal from the shared part of the exit path needs to be
> reflected on both of the sides of the JA, i.e. ...
>
>>
>> + /* Clear the supervisor shadow stack token busy bit. */
>> +.macro rag_clrssbsy
>> + rdsspq %rcx
>> + clrssbsy (%rcx)
>> +.endm
>> + ALTERNATIVE "", rag_clrssbsy, X86_FEATURE_XEN_SHSTK
>> +
>> + movq 8(%rsp), %rcx # RIP
> ... not just here, but also like this (with the JA above changed
> to target the new label):
>
> ALIGN
> /* No special register assumptions. */
> +.Liret_exit_to_guest:
> + movq 8(%rsp),%rcx # RIP
> iret_exit_to_guest:
> andl $~(X86_EFLAGS_IOPL|X86_EFLAGS_NT|X86_EFLAGS_VM),24(%rsp)
> orl $X86_EFLAGS_IF,24(%rsp)
>
> Granted it's mostly cosmetic, as the IRETQ ought to fault, but
> it's still a use of IRET in place of SYSRET, and hence we better
> get guest register state right. With this or a functionally
> identical adjustment (or a clarification on what makes you
> convinced this adjustment isn't needed)
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Ah yes. I really ought to retroactively create an XSA-7 PoC for this.
Will fix.
~Andrew
^ permalink raw reply
* [igt-dev] [PATCH i-g-t] lib/kms_chamelium: Wait for the sink to reconnect after an FSM DPMS-off
From: Imre Deak @ 2020-05-29 19:58 UTC (permalink / raw)
To: igt-dev; +Cc: Kunal Joshi
After Chamelium does an FSM signaling by deasserting its HPD, in
response to which the test's FSM handler disables the output with a
DPMS-off, we have to make sure that Chamelium has reasserted its HPD
before we re-enable the output with DPMS-on (for instance to avoid link
training errors, or enable the output in the wrong TypeC mode). To
ensure this wait for the connector state to become asserted.
On TypeC connectors with an enabled mode the IOM firmware will signal a
connected state (via a connect hotplug interrupt delivered to the
driver) in a deferred way only after the mode is disabled. So wait for
the connected state after DPMS-off.
Reported-and-tested-by: Kunal Joshi <kunal1.joshi@intel.com>
Cc: Hiler Arkadiusz <arkadiusz.hiler@intel.com>
Cc: Kunal Joshi <kunal1.joshi@intel.com>
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
lib/igt_chamelium.c | 47 ++++++++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 15 deletions(-)
diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index ff4644f8f..ed92bc782 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -243,6 +243,35 @@ struct fsm_monitor_args {
struct udev_monitor *mon;
};
+static bool wait_for_connected_state(int drm_fd,
+ int *connectors, int connector_count)
+{
+ igt_assert(connector_count > 0);
+
+ igt_until_timeout(CHAMELIUM_HOTPLUG_DETECTION_DELAY) {
+ bool connected;
+
+ for (int i = 0; i < connector_count; i++) {
+ drmModeConnector *connector =
+ drmModeGetConnector(drm_fd, connectors[i]);
+
+ connected = connector->connection == DRM_MODE_CONNECTED;
+
+ drmModeFreeConnector(connector);
+
+ if (!connected)
+ break;
+ }
+
+ if (connected)
+ return true;
+
+ usleep(50000);
+ }
+
+ return false;
+}
+
/*
* Whenever resolutions or other factors change with the display output, the
* Chamelium's display receivers need to be fully reset in order to perform any
@@ -272,6 +301,8 @@ static void *chamelium_fsm_mon(void *data)
connector = chamelium_port_get_connector(args->chamelium, args->port,
false);
kmstest_set_connector_dpms(drm_fd, connector, DRM_MODE_DPMS_OFF);
+ wait_for_connected_state(drm_fd, &args->port->connector_id, 1);
+
kmstest_set_connector_dpms(drm_fd, connector, DRM_MODE_DPMS_ON);
drmModeFreeConnector(connector);
@@ -2560,7 +2591,6 @@ bool chamelium_wait_all_configured_ports_connected(struct chamelium *chamelium,
drmModeConnector *connector;
char **group_list;
char *group;
- bool ret = true;
int connectors[CHAMELIUM_MAX_PORTS];
int connectors_count = 0;
@@ -2614,20 +2644,7 @@ bool chamelium_wait_all_configured_ports_connected(struct chamelium *chamelium,
return true;
}
- igt_until_timeout(CHAMELIUM_HOTPLUG_DETECTION_DELAY) {
- ret = true;
- for (int i = 0; i < connectors_count; ++i) {
- connector = drmModeGetConnector(drm_fd, connectors[i]);
- if (connector->connection != DRM_MODE_CONNECTED)
- ret = false;
- drmModeFreeConnector(connector);
- }
-
- if (ret)
- break;
- }
-
- return ret;
+ return wait_for_connected_state(drm_fd, connectors, connectors_count);
}
igt_constructor {
--
2.23.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related
* [Buildroot] [PATCH 1/1] package/matio: add upstream security fixes
From: Peter Korsgaard @ 2020-05-29 19:59 UTC (permalink / raw)
To: buildroot
In-Reply-To: <20200502195438.3358786-1-fontaine.fabrice@gmail.com>
>>>>> "Fabrice" == Fabrice Fontaine <fontaine.fabrice@gmail.com> writes:
> Fix the following CVEs:
> - CVE-2019-17533: Mat_VarReadNextInfo4 in mat4.c in MATIO 1.5.17 omits
> a certain '\0' character, leading to a heap-based buffer over-read in
> strdup_vprintf when uninitialized memory is accessed.
> - CVE-2019-20017: A stack-based buffer over-read was discovered in
> Mat_VarReadNextInfo5 in mat5.c in matio 1.5.17.
> - CVE-2019-20018: A stack-based buffer over-read was discovered in
> ReadNextCell in mat5.c in matio 1.5.17.
> - CVE-2019-20020: A stack-based buffer over-read was discovered in
> ReadNextStructField in mat5.c in matio 1.5.17.
> - CVE-2019-20052: A memory leak was discovered in Mat_VarCalloc in
> mat.c in matio 1.5.17 because SafeMulDims does not consider the
> rank==0 case.
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Committed, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply
* [PATCH] HID: usbhid: do not sleep when opening device
From: Dmitry Torokhov @ 2020-05-29 19:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: groeck, Nicolas Boichat, linux-usb, linux-input, linux-kernel
usbhid tries to give the device 50 milliseconds to drain its queues
when opening the device, but does it naively by simply sleeping in open
handler, which slows down device probing (and thus may affect overall
boot time).
However we do not need to sleep as we can instead mark a point of time
in the future when we should start processing the events.
Reported-by: Nicolas Boichat <drinkcat@chromium.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/usbhid/hid-core.c | 27 +++++++++++++++------------
drivers/hid/usbhid/usbhid.h | 1 +
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index c7bc9db5b192..e69992e945b2 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -95,6 +95,19 @@ static int hid_start_in(struct hid_device *hid)
set_bit(HID_NO_BANDWIDTH, &usbhid->iofl);
} else {
clear_bit(HID_NO_BANDWIDTH, &usbhid->iofl);
+
+ if (test_and_clear_bit(HID_RESUME_RUNNING,
+ &usbhid->iofl)) {
+ /*
+ * In case events are generated while nobody was
+ * listening, some are released when the device
+ * is re-opened. Wait 50 msec for the queue to
+ * empty before allowing events to go through
+ * hid.
+ */
+ usbhid->input_start_time = jiffies +
+ msecs_to_jiffies(50);
+ }
}
}
spin_unlock_irqrestore(&usbhid->lock, flags);
@@ -280,7 +293,8 @@ static void hid_irq_in(struct urb *urb)
if (!test_bit(HID_OPENED, &usbhid->iofl))
break;
usbhid_mark_busy(usbhid);
- if (!test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
+ if (!test_bit(HID_RESUME_RUNNING, &usbhid->iofl) &&
+ time_after(jiffies, usbhid->input_start_time)) {
hid_input_report(urb->context, HID_INPUT_REPORT,
urb->transfer_buffer,
urb->actual_length, 1);
@@ -714,17 +728,6 @@ static int usbhid_open(struct hid_device *hid)
}
usb_autopm_put_interface(usbhid->intf);
-
- /*
- * In case events are generated while nobody was listening,
- * some are released when the device is re-opened.
- * Wait 50 msec for the queue to empty before allowing events
- * to go through hid.
- */
- if (res == 0)
- msleep(50);
-
- clear_bit(HID_RESUME_RUNNING, &usbhid->iofl);
return res;
}
diff --git a/drivers/hid/usbhid/usbhid.h b/drivers/hid/usbhid/usbhid.h
index 8620408bd7af..805949671b96 100644
--- a/drivers/hid/usbhid/usbhid.h
+++ b/drivers/hid/usbhid/usbhid.h
@@ -82,6 +82,7 @@ struct usbhid_device {
spinlock_t lock; /* fifo spinlock */
unsigned long iofl; /* I/O flags (CTRL_RUNNING, OUT_RUNNING) */
+ unsigned long input_start_time; /* When to start handling input, in jiffies */
struct timer_list io_retry; /* Retry timer */
unsigned long stop_retry; /* Time to give up, in jiffies */
unsigned int retry_delay; /* Delay length in ms */
--
2.27.0.rc0.183.gde8f92d652-goog
--
Dmitry
^ permalink raw reply related
* Re: [net 3/6] net/mlx5e: Remove warning "devices are not on same switch HW"
From: Saeed Mahameed @ 2020-05-29 20:00 UTC (permalink / raw)
To: kuba@kernel.org
Cc: Roi Dayan, Maor Dickman, davem@davemloft.net,
netdev@vger.kernel.org
In-Reply-To: <20200529121222.4b68ce20@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
On Fri, 2020-05-29 at 12:12 -0700, Jakub Kicinski wrote:
> On Thu, 28 May 2020 23:56:42 -0700 Saeed Mahameed wrote:
> > From: Maor Dickman <maord@mellanox.com>
> >
> > On tunnel decap rule insertion, the indirect mechanism will attempt
> > to
> > offload the rule on all uplink representors which will trigger the
> > "devices are not on same switch HW, can't offload forwarding"
> > message
> > for the uplink which isn't on the same switch HW as the VF
> > representor.
> >
> > The above flow is valid and shouldn't cause warning message,
> > fix by removing the warning and only report this flow using extack.
> >
> > Fixes: f3953003a66f ("net/mlx5e: Fix allowed tc redirect merged
> > eswitch offload cases")
> > Signed-off-by: Maor Dickman <maord@mellanox.com>
> > Reviewed-by: Roi Dayan <roid@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
>
> Fixes tag: Fixes: f3953003a66f ("net/mlx5e: Fix allowed tc redirect
> merged eswitch offload cases")
> Has these problem(s):
> - Target SHA1 does not exist
Fixing, thanks !
^ permalink raw reply
* [PATCH 6/9] staging: media: atomisp: fix type mismatch
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
In-Reply-To: <20200529200031.4117841-1-arnd@arndb.de>
The caller passes a variable of a different enum type:
drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c:1707:64: error: implicit conversion from enumeration type 'const enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
binary_supports_input_format(xcandidate, req_in_info->format));
An earlier patch tried to address this by changing the type
of the function argument, but as the caller passes two different
arguments, there is still a warning.
Assume that the last patch was correct and change the other caller
as well.
Fixes: 0116b8df1c9e ("media: staging: atomisp: stop duplicating input format types")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
index 2a23b7c6aeeb..10d698295daf 100644
--- a/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
+++ b/drivers/staging/media/atomisp/pci/runtime/binary/src/binary.c
@@ -1704,7 +1704,7 @@ ia_css_binary_find(struct ia_css_binary_descr *descr,
ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,
"ia_css_binary_find() [%d] continue: !%d\n",
__LINE__,
- binary_supports_input_format(xcandidate, req_in_info->format));
+ binary_supports_input_format(xcandidate, descr->stream_format));
continue;
}
#endif
--
2.26.2
^ permalink raw reply related
* [PATCH 5/9] staging: media: atomisp: fix stack overflow in init_pipe_defaults()
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
In-Reply-To: <20200529200031.4117841-1-arnd@arndb.de>
When building with clang, multiple copies of the structures to be
initialized are passed around on the stack and copied locally, using an
insane amount of stack space:
drivers/staging/media/atomisp/pci/sh_css.c:2371:1: error: stack frame size of 26864 bytes in function 'create_pipe' [-Werror,-Wframe-larger-than=]
Use constantly-allocated variables plus an explicit memcpy()
to avoid that.
Fixes: 6dc9a2568f84 ("media: atomisp: convert default struct values to use compound-literals with designated initializers")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/sh_css.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index e91c6029c651..1e8b9d637116 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -2264,6 +2264,12 @@ static enum ia_css_err
init_pipe_defaults(enum ia_css_pipe_mode mode,
struct ia_css_pipe *pipe,
bool copy_pipe) {
+ static const struct ia_css_pipe default_pipe = IA_CSS_DEFAULT_PIPE;
+ static const struct ia_css_preview_settings preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
+ static const struct ia_css_capture_settings capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
+ static const struct ia_css_video_settings video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
+ static const struct ia_css_yuvpp_settings yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
+
if (!pipe)
{
IA_CSS_ERROR("NULL pipe parameter");
@@ -2271,14 +2277,14 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
}
/* Initialize pipe to pre-defined defaults */
- *pipe = IA_CSS_DEFAULT_PIPE;
+ memcpy(pipe, &default_pipe, sizeof(default_pipe));
/* TODO: JB should not be needed, but temporary backward reference */
switch (mode)
{
case IA_CSS_PIPE_MODE_PREVIEW:
pipe->mode = IA_CSS_PIPE_ID_PREVIEW;
- pipe->pipe_settings.preview = IA_CSS_DEFAULT_PREVIEW_SETTINGS;
+ memcpy(&pipe->pipe_settings.preview, &preview, sizeof(preview));
break;
case IA_CSS_PIPE_MODE_CAPTURE:
if (copy_pipe) {
@@ -2286,11 +2292,11 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
} else {
pipe->mode = IA_CSS_PIPE_ID_CAPTURE;
}
- pipe->pipe_settings.capture = IA_CSS_DEFAULT_CAPTURE_SETTINGS;
+ memcpy(&pipe->pipe_settings.capture, &capture, sizeof(capture));
break;
case IA_CSS_PIPE_MODE_VIDEO:
pipe->mode = IA_CSS_PIPE_ID_VIDEO;
- pipe->pipe_settings.video = IA_CSS_DEFAULT_VIDEO_SETTINGS;
+ memcpy(&pipe->pipe_settings.video, &video, sizeof(video));
break;
case IA_CSS_PIPE_MODE_ACC:
pipe->mode = IA_CSS_PIPE_ID_ACC;
@@ -2300,7 +2306,7 @@ init_pipe_defaults(enum ia_css_pipe_mode mode,
break;
case IA_CSS_PIPE_MODE_YUVPP:
pipe->mode = IA_CSS_PIPE_ID_YUVPP;
- pipe->pipe_settings.yuvpp = IA_CSS_DEFAULT_YUVPP_SETTINGS;
+ memcpy(&pipe->pipe_settings.yuvpp, &yuvpp, sizeof(yuvpp));
break;
default:
return IA_CSS_ERR_INVALID_ARGUMENTS;
--
2.26.2
^ permalink raw reply related
* [PATCH 3/9] staging: media: atomisp: annotate an unused function
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
In-Reply-To: <20200529200031.4117841-1-arnd@arndb.de>
atomisp_mrfld_power() has no more callers and produces
a warning:
drivers/staging/media/atomisp/pci/atomisp_v4l2.c:764:12: error: unused function 'atomisp_mrfld_power' [-Werror,-Wunused-function]
Mark the function as unused while the PM code is being
debugged, expecting that it will be used again in the
future and should not just be removed.
Fixes: 95d1f398c4dc ("media: atomisp: keep the ISP powered on when setting it")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 694268d133c0..10abb35ba0e0 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -761,7 +761,8 @@ static void punit_ddr_dvfs_enable(bool enable)
pr_info("DDR DVFS, door bell is not cleared within 3ms\n");
}
-static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
+static __attribute__((unused)) int
+atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
{
unsigned long timeout;
u32 val = enable ? MRFLD_ISPSSPM0_IUNIT_POWER_ON :
--
2.26.2
^ permalink raw reply related
* Re: [PATCH v2 13/14] x86/S3: Save and restore Shadow Stack configuration
From: Andrew Cooper @ 2020-05-29 20:00 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel, Wei Liu, Roger Pau Monné
In-Reply-To: <c1f1cb73-65f7-f2f7-161c-b505edc5959e@suse.com>
On 29/05/2020 13:52, Jan Beulich wrote:
> On 27.05.2020 21:18, Andrew Cooper wrote:
>> See code for details
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Wei Liu <wl@xen.org>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Semi-RFC - I can't actually test this path. Currently attempting to arrange
>> for someone else to.
> Nevertheless
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with one question, just for my understanding:
>
>> @@ -48,6 +58,51 @@ ENTRY(s3_resume)
>> pushq %rax
>> lretq
>> 1:
>> +#ifdef CONFIG_XEN_SHSTK
>> + /*
>> + * Restoring SSP is a little complicated, because we are intercepting
>> + * an in-use shadow stack. Write a temporary token under the stack,
>> + * so SETSSBSY will successfully load a value useful for us, then
>> + * reset MSR_PL0_SSP to its usual value and pop the temporary token.
>> + */
>> + mov saved_rsp(%rip), %rdi
>> + cmpq $1, %rdi
>> + je .L_shstk_done
>> +
>> + /* Set up MSR_S_CET. */
>> + mov $MSR_S_CET, %ecx
>> + xor %edx, %edx
>> + mov $CET_SHSTK_EN | CET_WRSS_EN, %eax
>> + wrmsr
>> +
>> + /* Construct the temporary supervisor token under SSP. */
>> + sub $8, %rdi
>> +
>> + /* Load it into MSR_PL0_SSP. */
>> + mov $MSR_PL0_SSP, %ecx
>> + mov %rdi, %rdx
>> + shr $32, %rdx
>> + mov %edi, %eax
>> + wrmsr
>> +
>> + /* Enable CET. MSR_INTERRUPT_SSP_TABLE is set up later in load_system_tables(). */
>> + mov $XEN_MINIMAL_CR4 | X86_CR4_CET, %ebx
>> + mov %rbx, %cr4
> Does this imply NMI or #MC are fatal between here and there?
Yes, but that is always the case during CPU bringup.
Only a few instructions ago, we didn't have an IDT, and we don't have
yet have an established %tr, so can't get the regular IST pointer either.
~Andrew
^ permalink raw reply
* [PATCH 9/9] staging: media: atomisp: add PMIC_OPREGION dependency
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
In-Reply-To: <20200529200031.4117841-1-arnd@arndb.de>
Without that driver, there is a link failure in
ERROR: modpost: "intel_soc_pmic_exec_mipi_pmic_seq_element"
[drivers/staging/media/atomisp/pci/atomisp_gmin_platform.ko] undefined!
Add an explicit Kconfig dependency.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/atomisp/Kconfig b/drivers/staging/media/atomisp/Kconfig
index c4f3049b0706..e86311c14329 100644
--- a/drivers/staging/media/atomisp/Kconfig
+++ b/drivers/staging/media/atomisp/Kconfig
@@ -11,6 +11,7 @@ menuconfig INTEL_ATOMISP
config VIDEO_ATOMISP
tristate "Intel Atom Image Signal Processor Driver"
depends on VIDEO_V4L2 && INTEL_ATOMISP
+ depends on PMIC_OPREGION
select IOSF_MBI
select VIDEOBUF_VMALLOC
---help---
--
2.26.2
^ permalink raw reply related
* [PATCH 2/9] staging: media: atomisp: declare 'struct device' before using it
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
In-Reply-To: <20200529200031.4117841-1-arnd@arndb.de>
In some configurations, including this header leads to a warning:
drivers/staging/media/atomisp//pci/sh_css_firmware.h:41:38: error: declaration of 'struct device' will not be visible outside of this function [-Werror,-Wvisibility]
Make sure the struct tag is known before declaring a function
that uses it as an argument.
Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/sh_css_firmware.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/atomisp/pci/sh_css_firmware.h b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
index f6253392a6c9..317559c7689f 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_firmware.h
+++ b/drivers/staging/media/atomisp/pci/sh_css_firmware.h
@@ -37,6 +37,7 @@ extern unsigned int sh_css_num_binaries;
char
*sh_css_get_fw_version(void);
+struct device;
bool
sh_css_check_firmware_version(struct device *dev, const char *fw_data);
--
2.26.2
^ permalink raw reply related
* [PATCH 1/9] staging: media: atomisp: fix incorrect NULL pointer check
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
Checking the pointer to a member of a struct against NULL
is pointless as clang points out:
drivers/staging/media/atomisp/pci/atomisp_cmd.c:4278:17: error: address of 'config->info' will always evaluate to 'true'
Check the original pointer instead, which may also be
unnecessary here, but makes a little more sense.
Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/atomisp_cmd.c | 2 +-
drivers/staging/media/atomisp/pci/sh_css.c | 4 ++--
drivers/staging/media/atomisp/pci/sh_css_sp.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 5be690f876c1..342fc3b34fe0 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -4275,7 +4275,7 @@ int atomisp_param(struct atomisp_sub_device *asd, int flag,
atomisp_css_get_dvs_grid_info(
&asd->params.curr_grid_info);
- if (!&config->info) {
+ if (!config) {
dev_err(isp->dev, "ERROR: NULL pointer in grid_info\n");
return -EINVAL;
}
diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
index d77432254a2c..e91c6029c651 100644
--- a/drivers/staging/media/atomisp/pci/sh_css.c
+++ b/drivers/staging/media/atomisp/pci/sh_css.c
@@ -8534,7 +8534,7 @@ ia_css_pipe_load_extension(struct ia_css_pipe *pipe,
if (firmware->info.isp.type == IA_CSS_ACC_OUTPUT)
{
- if (&pipe->output_stage)
+ if (pipe)
append_firmware(&pipe->output_stage, firmware);
else {
IA_CSS_LEAVE_ERR_PRIVATE(IA_CSS_ERR_INTERNAL_ERROR);
@@ -8542,7 +8542,7 @@ ia_css_pipe_load_extension(struct ia_css_pipe *pipe,
}
} else if (firmware->info.isp.type == IA_CSS_ACC_VIEWFINDER)
{
- if (&pipe->vf_stage)
+ if (pipe)
append_firmware(&pipe->vf_stage, firmware);
else {
IA_CSS_LEAVE_ERR_PRIVATE(IA_CSS_ERR_INTERNAL_ERROR);
diff --git a/drivers/staging/media/atomisp/pci/sh_css_sp.c b/drivers/staging/media/atomisp/pci/sh_css_sp.c
index e574396ad0f4..c0e579c1705f 100644
--- a/drivers/staging/media/atomisp/pci/sh_css_sp.c
+++ b/drivers/staging/media/atomisp/pci/sh_css_sp.c
@@ -1022,7 +1022,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
if (!pipe)
return IA_CSS_ERR_INTERNAL_ERROR;
ia_css_get_crop_offsets(pipe, &args->in_frame->info);
- } else if (&binary->in_frame_info)
+ } else if (binary)
{
pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
if (!pipe)
@@ -1036,7 +1036,7 @@ sh_css_sp_init_stage(struct ia_css_binary *binary,
if (!pipe)
return IA_CSS_ERR_INTERNAL_ERROR;
ia_css_get_crop_offsets(pipe, &args->in_frame->info);
- } else if (&binary->in_frame_info) {
+ } else if (binary) {
pipe = find_pipe_by_num(sh_css_sp_group.pipe[thread_id].pipe_num);
if (!pipe)
return IA_CSS_ERR_INTERNAL_ERROR;
--
2.26.2
^ permalink raw reply related
* [PATCH 7/9] staging: media: atomisp: fix enum type mixups
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
In-Reply-To: <20200529200031.4117841-1-arnd@arndb.de>
Some function calls pass an incorrect enum type:
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:858:16: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
gp_device_rst(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:860:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_switch_rst(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:876:27: error: implicit conversion from enumeration type 'input_system_cfg_flag_t' to different enumeration type 'input_system_connection_t' [-Werror,-Wenum-conversion]
config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1326:32: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c:1329:19: error: implicit conversion from enumeration type 'input_system_ID_t' to different enumeration type 'gp_device_ID_t' [-Werror,-Wenum-conversion]
input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
INPUT_SYSTEM0_ID is zero, so use the corresponding zero-value
of the expected types instead.
Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
.../pci/hive_isp_css_common/host/input_system.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
index 2114cf4f3fda..aa0f0fca9346 100644
--- a/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
+++ b/drivers/staging/media/atomisp/pci/hive_isp_css_common/host/input_system.c
@@ -855,9 +855,9 @@ input_system_error_t input_system_configuration_reset(void)
input_system_network_rst(INPUT_SYSTEM0_ID);
- gp_device_rst(INPUT_SYSTEM0_ID);
+ gp_device_rst(GP_DEVICE0_ID);
- input_switch_rst(INPUT_SYSTEM0_ID);
+ input_switch_rst(GP_DEVICE0_ID);
//target_rst();
@@ -873,7 +873,7 @@ input_system_error_t input_system_configuration_reset(void)
for (i = 0; i < N_CSI_PORTS; i++) {
config.csi_buffer_flags[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
- config.multicast[i] = INPUT_SYSTEM_CFG_FLAG_RESET;
+ config.multicast[i] = INPUT_SYSTEM_DISCARD_ALL;
}
config.source_type_flags = INPUT_SYSTEM_CFG_FLAG_RESET;
@@ -1323,10 +1323,10 @@ static input_system_error_t configuration_to_registers(void)
} // end of switch (source_type)
// Set input selector.
- input_selector_cfg_for_sensor(INPUT_SYSTEM0_ID);
+ input_selector_cfg_for_sensor(GP_DEVICE0_ID);
// Set input switch.
- input_switch_cfg(INPUT_SYSTEM0_ID, &config.input_switch_cfg);
+ input_switch_cfg(GP_DEVICE0_ID, &config.input_switch_cfg);
// Set input formatters.
// AM: IF are set dynamically.
--
2.26.2
^ permalink raw reply related
* [Intel-wired-lan] [next-queue v2] i40e: Add support for 5Gbps cards
From: Jeff Kirsher @ 2020-05-29 20:01 UTC (permalink / raw)
To: intel-wired-lan
From: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Make possible for the i40e driver to bind to the new v710 for 5GBASE-T
NICs.
Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_common.c | 3 +++
drivers/net/ethernet/intel/i40e/i40e_devids.h | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index 4ab081953e19..afad5e9f80e0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -27,6 +27,7 @@ i40e_status i40e_set_mac_type(struct i40e_hw *hw)
case I40E_DEV_ID_QSFP_A:
case I40E_DEV_ID_QSFP_B:
case I40E_DEV_ID_QSFP_C:
+ case I40E_DEV_ID_5G_BASE_T_BC:
case I40E_DEV_ID_10G_BASE_T:
case I40E_DEV_ID_10G_BASE_T4:
case I40E_DEV_ID_10G_BASE_T_BC:
@@ -4906,6 +4907,7 @@ i40e_status i40e_write_phy_register(struct i40e_hw *hw,
status = i40e_write_phy_register_clause22(hw, reg, phy_addr,
value);
break;
+ case I40E_DEV_ID_5G_BASE_T_BC:
case I40E_DEV_ID_10G_BASE_T:
case I40E_DEV_ID_10G_BASE_T4:
case I40E_DEV_ID_10G_BASE_T_BC:
@@ -4943,6 +4945,7 @@ i40e_status i40e_read_phy_register(struct i40e_hw *hw,
status = i40e_read_phy_register_clause22(hw, reg, phy_addr,
value);
break;
+ case I40E_DEV_ID_5G_BASE_T_BC:
case I40E_DEV_ID_10G_BASE_T:
case I40E_DEV_ID_10G_BASE_T4:
case I40E_DEV_ID_10G_BASE_T_BC:
diff --git a/drivers/net/ethernet/intel/i40e/i40e_devids.h b/drivers/net/ethernet/intel/i40e/i40e_devids.h
index 33df3bf2f73b..1bcb0ec0f0c0 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_devids.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_devids.h
@@ -23,8 +23,10 @@
#define I40E_DEV_ID_10G_BASE_T_BC 0x15FF
#define I40E_DEV_ID_10G_B 0x104F
#define I40E_DEV_ID_10G_SFP 0x104E
+#define I40E_DEV_ID_5G_BASE_T_BC 0x101F
#define I40E_IS_X710TL_DEVICE(d) \
- ((d) == I40E_DEV_ID_10G_BASE_T_BC)
+ (((d) == I40E_DEV_ID_5G_BASE_T_BC) || \
+ ((d) == I40E_DEV_ID_10G_BASE_T_BC))
#define I40E_DEV_ID_KX_X722 0x37CE
#define I40E_DEV_ID_QSFP_X722 0x37CF
#define I40E_DEV_ID_SFP_X722 0x37D0
--
2.26.2
^ permalink raw reply related
* [PATCH 8/9] staging: media: atomisp: disable all custom formats
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
In-Reply-To: <20200529200031.4117841-1-arnd@arndb.de>
clang points out the usage of an incorrect enum type in the
list of supported image formats:
drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:65: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
drivers/staging/media/atomisp/pci/atomisp_subdev.c:49:39: error: implicit conversion from enumeration type 'enum ia_css_frame_format' to different enumeration type 'enum atomisp_input_format' [-Werror,-Wenum-conversion]
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 },
{ MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
Checking the git history, I found a commit that disabled one such case
because it did not work. It seems likely that the incorrect enum was
part of the original problem and that the others do not work either,
or have never been tested.
Disable all the ones that cause a warning.
Fixes: cb02ae3d71ea ("media: staging: atomisp: Disable custom format for now")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/staging/media/atomisp/pci/atomisp_subdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 46590129cbe3..8bce466cc128 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -44,9 +44,11 @@ const struct atomisp_in_fmt_conv atomisp_in_fmt_conv[] = {
{ MEDIA_BUS_FMT_SRGGB12_1X12, 12, 12, ATOMISP_INPUT_FORMAT_RAW_12, CSS_BAYER_ORDER_RGGB, CSS_FORMAT_RAW_12 },
{ MEDIA_BUS_FMT_UYVY8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0, ATOMISP_INPUT_FORMAT_YUV422_8 },
{ MEDIA_BUS_FMT_YUYV8_1X16, 8, 8, ATOMISP_INPUT_FORMAT_YUV422_8, 0, ATOMISP_INPUT_FORMAT_YUV422_8 },
+#if 0
{ MEDIA_BUS_FMT_JPEG_1X8, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
{ V4L2_MBUS_FMT_CUSTOM_NV12, 12, 12, CSS_FRAME_FORMAT_NV12, 0, CSS_FRAME_FORMAT_NV12 },
{ V4L2_MBUS_FMT_CUSTOM_NV21, 12, 12, CSS_FRAME_FORMAT_NV21, 0, CSS_FRAME_FORMAT_NV21 },
+#endif
{ V4L2_MBUS_FMT_CUSTOM_YUV420, 12, 12, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY, 0, ATOMISP_INPUT_FORMAT_YUV420_8_LEGACY },
#if 0
{ V4L2_MBUS_FMT_CUSTOM_M10MO_RAW, 8, 8, CSS_FRAME_FORMAT_BINARY_8, 0, ATOMISP_INPUT_FORMAT_BINARY_8 },
--
2.26.2
^ permalink raw reply related
* [PATCH 4/9] staging: media: atomisp: fix a type conversion warning
From: Arnd Bergmann @ 2020-05-29 20:00 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Sakari Ailus, linux-media, devel, linux-kernel, clang-built-linux,
Arnd Bergmann
In-Reply-To: <20200529200031.4117841-1-arnd@arndb.de>
clang complains that the type conversion in the MAX() macro
contains an implied integer overflow to a signed number:
drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c:129:35:
error: implicit conversion from 'unsigned long' to 'int32_t' (aka 'int') changes value from 18446744073709543424 to -8192 [-Werror,-Wconstant-conversion]
As the conversion is clearly intended here, use an explicit cast.
Fixes: 9a0d7fb5ece6 ("media: atomisp: simplify math_support.h")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
.../atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c b/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c
index a9db6366d20b..613bace0522a 100644
--- a/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c
+++ b/drivers/staging/media/atomisp/pci/isp/kernels/xnr/xnr_3.0/ia_css_xnr3.host.c
@@ -126,7 +126,7 @@ compute_blending(int strength)
* exactly as s0.11 fixed point, but -1.0 can.
*/
isp_strength = -(((strength * isp_scale) + offset) / host_scale);
- return MAX(MIN(isp_strength, 0), -XNR_BLENDING_SCALE_FACTOR);
+ return MAX(MIN(isp_strength, 0), -(unsigned int)XNR_BLENDING_SCALE_FACTOR);
}
void
--
2.26.2
^ permalink raw reply related
* Re: Some -serious- BPF-related litmus tests
From: Andrii Nakryiko @ 2020-05-29 20:01 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Joel Fernandes, Paul E. McKenney, Andrii Nakryiko, Alan Stern,
parri.andrea, will, Boqun Feng, npiggin, dhowells, j.alglave,
luc.maranget, Akira Yokosawa, dlustig, open list, linux-arch
In-Reply-To: <20200529123626.GL706495@hirez.programming.kicks-ass.net>
On Fri, May 29, 2020 at 5:36 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Thu, May 28, 2020 at 10:14:21PM -0700, Andrii Nakryiko wrote:
>
> > There is another cluster of applications which are unnecessarily more
> > complicated just for the fact that there is no ordering between
> > correlated events that happen on different CPUs. Per-CPU buffers are
> > not well suited for such cases, unfortunately.
>
> And yet, I've been debugging concurrency issues with ftrace for well
> over a decade.
>
> In fact, adding a spinlock in the mix will make a certain class of
> concurrency problems go-away! because you introduce artificial
> serialization between CPUs.
>
> Heck, even the ftrace buffer can sometimes make problems go way just
> because of the overhead of tracing itself changes the timing.
>
All true, but seems like you are looking at this from purely tracing
perspective, especially with very high-frequency events. And that's a
very challenging domain for sure. But *a lot* of BPF applications
(actually all I'm aware of at Facebook and outside of it) trace and
collect much less high-frequency events, so they don't need to
sacrifice so much to get ultimate performance. Performance profiling,
which comes closest to what you are describing, is just one of many
uses.
> It is perfectly possible to reconstruct order with per-cpu buffers in so
> far as that there is order to begin with. Esp on modern CPUs that have
> synchronized TSC.
>
> Computers are not SC, pretending that events are is just a lie.
Right, with precise enough clock or some atomic in-kernel counter, you
can re-construct order in user-space. But that has its own downsides:
you need to send this counter over the wire with every sample (takes
more space, reducing amounf of "useful" payload you can fit in a ring
buffer), plus user-space re-sorting takes engineering effort and isn't
exactly trivial. Now multiply that by many teams who need this, and it
becomes a problem worth solving.
Few examples of events that do occur sequentially in an ordered
manner, but due to per-cpu buffers might come back to user-space out
of order: fork/exec/exit events and tcp connection state tracking.
E.g., for short-lived process, fork can happen on one CPU, exit - on
another within tiny period of time between each other. In such case,
user-space might get exit event from one buffer before getting a fork
event on another one. Similarly for TCP connection state transitions.
For fork/exec/exit, typical rate of events, even on 80-core machines
is on the order of thousands of events per second at most (typically,
spikes do happen, unfortunately), so MPSC queue contention isn't a big
deal.
>
> > > people, but apparently they've not spend enough time debugging stuff
> > > with partial logs yet. Of course, bpf_prog_active already makes BPF
> > > lossy, so maybe they went with that.
> >
> > Not sure which "partial logs" you mean, I'll assume dropped samples?
>
> Yep. Trying to reconstruct what actually happened from incomplete logs
> is one of the most frustrating things in the world.
>
> > All production BPF applications are already designed to handle data
> > loss, because it could and does happen due to running out of buffer
> > space. Of course, amount of such drops is minimized however possible,
> > but applications still need to be able to handle that.
>
> Running out of space is fixable and 'obvious'. Missing random events in
> the middle is bloody infuriating. Even more so if you can't tell there's
> gaps in the midle.
>
> AFAICT, you don't even mark a reservation fail.... ah, you leave that to
> the user :-( And it can't tell if it's spurious or space related.
BPF program cannot ignore reservation failure, it has to check that
reserve() returned non-NULL pointer, verifier enforces that. It's true
that out-of-space vs locking failed in NMI is indistinguishable. We'll
see how important it is to distinguish in practice, there are very few
applications that do run in NMI context.
As for internal accounting of dropped samples, I've considered that
and there is 4 bytes per-sample I can use to communicate it back to
user-space, but it requires another atomic increment, which would just
reduce performance further. Modern BPF applications actually have a
very simple and straightforward ways to count that themselves with use
of global variables, memory-mapped into user-space. So it's simple and
fast to do. But again, we'll see in practice how that works for users
and will adjust/enhance as necessary.
>
> Same with bpf_prog_active, it's silent 'random' data loss. You can
> easily tell where a CPU buffer starts and stops, and thus if the events
> are contained within, but not if there's random bits missing from the
> middle.
>
> > Now, though, there will be more options. Now it's not just a question
> > of whether to allocate a tiny 64KB per-CPU buffer on 80 core server
> > and use reasonable 5MB for perfbuf overall, but suffer high and
> > frequent data loss whenever a burst of incoming events happen. Or bump
> > it up to, say, 256KB (or higher) and use 20MB+ now, which most of the
> > time will be completely unused, but be able to absorb 4x more events.
> > Now it might be more than enough to just allocate a single shared 5MB
> > buffer and be able to absorb much higher spikes (of course, assuming
> > not all CPUs will be spiking at the same time, in which case nothing
> > can really help you much w.r.t. data loss).
>
> Muwhahaha, a single shared buffer with 80 CPUs! That's bloody murder on
> performance.
Loved the laugh :) But see above, a lot of interesting events are
pretty low-frequency even on those giant servers.
>
> > So many BPF users are acutely aware of data loss and care a lot, but
> > there are other constraints that they have to take into account.
> >
> > As for expensiveness of spinlock and atomics, a lot of applications we
> > are seeing do not require huge throughput that per-CPU data structures
> > provide, so such overhead is acceptable. Even under high contention,
> > BPF ringbuf performance is pretty reasonable and will satisfy a lot of
> > applications, see [1].
>
> I've done benchmarks on contended atomic ops, and they go from ~20
> cycles (uncontended, cache hot) to well over 10k cycles when you jump on
> them with say a dozen cores across a few nodes (more numa, more
> horrible).
>
> > [1] https://patchwork.ozlabs.org/project/netdev/patch/20200526063255.1675186-5-andriin@fb.com/
>
> From that: "Ringbuf, multi-producer contention", right? I read that as:
> 'performance is bloody horrible if you add contention'.
>
> I suppose most of your users have very low event rates, otherwise I
> can't see that working.
Yes and yes :) Good thing is that with MPSC ringbuf, if contention is
an issue, they can go with a model of 1 ringbuf for each cpu, similar
to perfbuf, or something in between with few ringbufs for larger
number of CPUs, again trading performance for memory, as necessary.
This is easy to do in BPF by combining ringbuf map with ARRAY_OF_MAPS
or HASH_OF_MAPS.
>
> > > All reasons why I never bother with BPF, aside from it being more
> > > difficult than hacking up a kernel in the first place.
> >
> > It's not my goal to pitch BPF here, but for a lot of real-world use
> > cases, hacking kernel is not an option at all, for many reasons. One
> > of them is that kernel upgrades across huge fleet of servers take a
> > long time, which teams can't afford to wait. In such cases, BPF is a
> > perfect solution, which can't be beaten, as evidenced by a wide
> > variety of BPF applications solving real problems.
>
> Yeah; lots of people use it because they really have nothing better for
> their situation.
>
> As long as people understand the constraints (and that's a *BIG* if) I
> suppose it's usable.
>
> It's just things I don't want to have to worry about.
>
> Anyway, all that said, I like how you did the commits, I should look to
> see if I can retro-fit the perf buffer to have some of that. Once
Thanks, and yeah, it would be actually great to have this
reserve/commit API for perfbuf as well. Internally it actually does
that, AFAIU, but all of that is enclosed in
preempt_disable/preempt_enable region, not sure how practical and easy
it is to split this up and let BPF verifier enforce that each
reservation is followed by commit. Having this kind of API would allow
to eliminate some unfortunate code patterns with using extra per-CPU
array just to prepare a record before sending it over perfbuf with
perf_event_output().
> question though; why are you using xchg() for the commit? Isn't that
> more expensive than it should be?
>
> That is, why isn't that:
>
> smp_store_release(&hdr->len, new_len);
>
> ? Or are you needing the smp_mb() for the store->load ordering for the
> ->consumer_pos load? That really needs a comment.
Yeah, smp_store_release() is not strong enough, this memory barrier is
necessary. And yeah, I'll follow up with some more comments, that's
been what Joel requested as well.
>
> I think you can get rid of the smp_load_acquire() there, you're ordering
> a load->store and could rely on the branch to do that:
>
> cons_pos = READ_ONCE(&rb->consumer_pos) & rb->mask;
> if ((flags & BPF_RB_FORCE_WAKEUP) || (cons_pos == rec_pos && !(flags &BPF_RB_NO_WAKEUP))
> irq_work_queue(&rq->work);
>
> should be a control dependency.
Could be. I tried to keep consistent
smp_load_acquire/smp_store_release usage to keep it simpler. It might
not be the absolutely minimal amount of ordering that would still be
correct. We might be able to tweak and tune this without changing
correctness.
Anyways, thanks for taking a look and feedback. I hope some of my
examples explain why this might be a fine approach for a lot of
applications, even if not the most performant one.
^ permalink raw reply
* Re: [PATCH v7 1/4] bitops: Introduce the the for_each_set_clump macro
From: Syed Nayyar Waris @ 2020-05-29 20:02 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Linus Walleij, Andrew Morton, William Breathitt Gray,
Arnd Bergmann, Linux-Arch, Linux Kernel Mailing List
In-Reply-To: <20200529183824.GW1634618@smile.fi.intel.com>
On Sat, May 30, 2020 at 12:08 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, May 29, 2020 at 11:38:18PM +0530, Syed Nayyar Waris wrote:
> > On Sun, May 24, 2020 at 8:15 PM kbuild test robot <lkp@intel.com> wrote:
>
> ...
>
> > > 579 static inline unsigned long bitmap_get_value(const unsigned long *map,
> > > 580 unsigned long start,
> > > 581 unsigned long nbits)
> > > 582 {
> > > 583 const size_t index = BIT_WORD(start);
> > > 584 const unsigned long offset = start % BITS_PER_LONG;
> > > 585 const unsigned long ceiling = roundup(start + 1, BITS_PER_LONG);
> > > 586 const unsigned long space = ceiling - start;
> > > 587 unsigned long value_low, value_high;
> > > 588
> > > 589 if (space >= nbits)
> > > > 590 return (map[index] >> offset) & GENMASK(nbits - 1, 0);
> > > 591 else {
> > > 592 value_low = map[index] & BITMAP_FIRST_WORD_MASK(start);
> > > 593 value_high = map[index + 1] & BITMAP_LAST_WORD_MASK(start + nbits);
> > > 594 return (value_low >> offset) | (value_high << space);
> > > 595 }
> > > 596 }
>
> > Regarding the above compilation warnings. All the warnings are because
> > of GENMASK usage in my patch.
> > The warnings are coming because of sanity checks present for 'GENMASK'
> > macro in include/linux/bits.h.
> >
> > Taking the example statement (in my patch) where compilation warning
> > is getting reported:
> > return (map[index] >> offset) & GENMASK(nbits - 1, 0);
> >
> > 'nbits' is of type 'unsigned long'.
> > In above, the sanity check is comparing '0' with unsigned value. And
> > unsigned value can't be less than '0' ever, hence the warning.
> > But this warning will occur whenever there will be '0' as one of the
> > 'argument' and an unsigned variable as another 'argument' for GENMASK.
> >
> > This warning is getting cleared if I cast the 'nbits' to 'long'.
> >
> > Let me know if I should submit a next patch with the casts applied.
> > What do you guys think?
>
> Proper fix is to fix GENMASK(), but allowed workaround is to use
> (BIT(nbits) - 1)
> instead.
>
> --
> With Best Regards,
> Andy Shevchenko
>
Hi Andy. Thank You for your comment.
When I used BIT macro (earlier), I had faced a problem. I want to tell
you about that.
Inside functions 'bitmap_set_value' and 'bitmap_get_value' when nbits (or
clump size) is BITS_PER_LONG, unexpected calculation happens.
Explanation:
Actually when nbits (clump size) is 64 (BITS_PER_LONG is 64 on my computer),
(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 happening.
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.
Since this macro is being 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).
William also knows about this issue:
"This is undefined behavior in the C standard (section 6.5.7 in the N1124)"
Andy, William,
Let me know what do you think ?
Regards
Syed Nayyar Waris
^ permalink raw reply
* Re: pull request (net-next): ipsec-next 2020-05-29
From: David Miller @ 2020-05-29 20:03 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
In-Reply-To: <20200529103011.30127-1-steffen.klassert@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 29 May 2020 12:30:00 +0200
> 1) Add IPv6 encapsulation support for ESP over UDP and TCP.
> From Sabrina Dubroca.
>
> 2) Remove unneeded reference when initializing xfrm interfaces.
> From Nicolas Dichtel.
>
> 3) Remove some indirect calls from the state_afinfo.
> From Florian Westphal.
>
> Please note that this pull request has two merge conflicts
...
> Both conflicts can be resolved as done in linux-next.
>
> Please pull or let me know if there are problems.
Pulled, thanks Steffen.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.