[PATCH 2/3] x509: Add support for NIST p192 keys in certificates and akcipher
From: Stefan Berger
Add support for NIST p192 keys in x509 certificates and support it in
'akcipher'.
Signed-off-by: Stefan Berger
---
crypto/asymmetric_keys/public_key.c | 3 ++
crypto/asymmetric_keys/x509_cert_parser.c | 1 +
crypto/ecc.c | 36 ++-
include/linux/oid_registry.h | 1 +
4 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/crypto/asymmetric_keys/public_key.c
b/crypto/asymmetric_keys/public_key.c
index 0fcbaec0ded0..bb4a7cc0e3c8 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -98,6 +98,9 @@ int software_key_determine_akcipher(const char *encoding,
oid = look_up_OID(pkey->params + 2, pkey->paramlen - 2);
switch (oid) {
+ case OID_id_prime192v1:
+ strcpy(alg_name, "nist_p192");
+ return 0;
case OID_id_prime256v1:
strcpy(alg_name, "nist_p256");
return 0;
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c
b/crypto/asymmetric_keys/x509_cert_parser.c
index 44bae5ccb475..720cc7977077 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -501,6 +501,7 @@ int x509_extract_key_data(void *context, size_t hdrlen,
enum OID oid = look_up_OID(ctx->params + 2,
ctx->params_size - 2);
switch (oid) {
+ case OID_id_prime192v1:
case OID_id_prime256v1:
ctx->cert->pub->pkey_algo = "ecdsa";
break;
diff --git a/crypto/ecc.c b/crypto/ecc.c
index fb8370720350..79df35a23a61 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -1826,13 +1826,47 @@ static struct akcipher_alg ecc_nist_p256 = {
},
};
+static unsigned int ecc_nist_p192_max_size(struct crypto_akcipher *tfm)
+{
+ return NIST_P192_KEY_SIZE;
+}
+
+static int ecc_nist_p192_init_tfm(struct crypto_akcipher *tfm)
+{
+ struct ecc_ctx *ctx = akcipher_tfm_ctx(tfm);
+
+ return ecc_ec_ctx_init(ctx, ECC_CURVE_NIST_P192);
+}
+
+static struct akcipher_alg ecc_nist_p192 = {
+ .verify = ecdsa_verify,
+ .set_pub_key = ecc_set_pub_key,
+ .max_size = ecc_nist_p192_max_size,
+ .init = ecc_nist_p192_init_tfm,
+ .exit = ecc_exit_tfm,
+ .base = {
+ .cra_name = "nist_p192",
+ .cra_driver_name = "ecc-nist-p192",
+ .cra_priority = 100,
+ .cra_module = THIS_MODULE,
+ .cra_ctxsize = sizeof(struct ecc_ctx),
+ },
+};
+
static int ecc_init(void)
{
- return crypto_register_akcipher(_nist_p256);
+ int ret;
+
+ ret = crypto_register_akcipher(_nist_p256);
+ if (ret)
+ return ret;
+
+ return crypto_register_akcipher(_nist_p192);
}
static void ecc_exit(void)
{
+ crypto_unregister_akcipher(_nist_p192);
crypto_unregister_akcipher(_nist_p256);
}
diff --git a/include/linux/oid_registry.h b/include/linux/oid_registry.h
index 9060f19c80eb..e8071133d0e2 100644
--- a/include/linux/oid_registry.h
+++ b/include/linux/oid_registry.h
@@ -21,6 +21,7 @@ enum OID {
OID_id_dsa, /* 1.2.840.10040.4.1 */
OID_id_ecdsa_with_sha1, /* 1.2.840.10045.4.1 */
OID_id_ecPublicKey, /* 1.2.840.10045.2.1 */
+ OID_id_prime192v1, /* 1.2.840.10045.3.1.1 */
OID_id_prime256v1, /* 1.2.840.10045.3.1.7 */
OID_id_ecdsa_with_sha224, /* 1.2.840.10045.4.3.1 */
OID_id_ecdsa_with_sha256, /* 1.2.840.10045.4.3.2 */
--
2.25.4
[PATCH v2] tpm: ibmvtpm: fix error return code in tpm_ibmvtpm_probe()
From: Stefan Berger
Return error code -ETIMEDOUT rather than '0' when waiting for the
rtce_buf to be set has timed out.
Fixes: d8d74ea3c002 ("tpm: ibmvtpm: Wait for buffer to be set before
proceeding")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
Signed-off-by: Stefan Berger
---
drivers/char/tpm/tpm_ibmvtpm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 994385bf37c0..813eb2cac0ce 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -687,6 +687,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
ibmvtpm->rtce_buf != NULL,
HZ)) {
dev_err(dev, "CRQ response timed out\n");
+ rc = -ETIMEDOUT;
goto init_irq_cleanup;
}
--
2.25.4
RE: [EXT] Re: [PATCH v2 RFC net-next 08/18] net: mvpp2: add FCA periodic timer configurations
> > > > > > > > > -- On Sun, Jan 24, 2021 at 01:43:57PM +0200, stef...@marvell.com > > > wrote: > > > > +/* Set Flow Control timer x140 faster than pause quanta to ensure > > > > +that link > > > > + * partner won't send taffic if port in XOFF mode. > > > > > > Can you explain more why 140 times faster is desirable here? Why 140 > > > times and not, say, 10 times faster? Where does this figure come > > > from, and what is the reasoning? Is there a switch that requires it? > > > > I tested with 140. > > Actually regarding to spec each quanta should be equal to 512 bit times. > > In 10G bit time is 0.1ns. > > And if the link has been negotiated to 10Mbps? Or is the clock already scaled > to the link speed? > >Andrew Currently its static, probably I can add function that reconfigure timer during runtime(based on link speed). Should it be part of this series or add it afterwards? Regards, Stefan.
RE: [EXT] Re: [PATCH v2 RFC net-next 04/18] net: mvpp2: add PPv23 version definition
> > We cannot access PPv2 register space before enabling clocks(done in > mvpp2_probe) , PP21 and PP22/23 have different sets of clocks. > > > So diff between PP21 and PP22/23 should be stored in device tree(in > > of_device_id), with MVPP22 and MVPP21 stored as .data > > Hi Stefan > > As far as i can see, you are not adding a new compatible. So 'marvell,armada- > 7k-pp2' means PPv2.2 and PPv2.3? It would be good to update the comment > at the beginning of marvell-pp2.txt to indicate this. > > Andrew Ok, I would add comment. Thanks, Stefan.
RE: [EXT] Re: [PATCH v2 RFC net-next 03/18] net: mvpp2: add CM3 SRAM memory map
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -91,6 +92,18 @@ static inline u32 mvpp2_cpu_to_thread(struct
> mvpp2 *priv, int cpu)
> > return cpu % priv->nthreads;
> > }
> >
> > +static inline
> > +void mvpp2_cm3_write(struct mvpp2 *priv, u32 offset, u32 data) {
> > + writel(data, priv->cm3_base + offset); }
> > +
> > +static inline
> > +u32 mvpp2_cm3_read(struct mvpp2 *priv, u32 offset) {
> > + return readl(priv->cm3_base + offset); }
>
> No inline functions in .c file please. Let the compiler decide.
>
>Andrew
Ok, I would fix.
Thanks,
Stefan.
RE: [EXT] Re: [PATCH v2 RFC net-next 08/18] net: mvpp2: add FCA periodic timer configurations
> > -- > On Sun, Jan 24, 2021 at 01:43:57PM +0200, stef...@marvell.com wrote: > > +/* Set Flow Control timer x140 faster than pause quanta to ensure > > +that link > > + * partner won't send taffic if port in XOFF mode. > > Can you explain more why 140 times faster is desirable here? Why 140 times > and not, say, 10 times faster? Where does this figure come from, and what is > the reasoning? Is there a switch that requires it? I tested with 140. Actually regarding to spec each quanta should be equal to 512 bit times. In 10G bit time is 0.1ns. So It actually should be: FC_CLK_DIVIDER = 1 / 512 = ~20. I took some buffer and made it 140. So maybe I can do it 100? Regards, Stefan.
RE: [EXT] Re: [PATCH v2 RFC net-next 04/18] net: mvpp2: add PPv23 version definition
> > Signed-off-by: Stefan Chulski > > --- > > drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 24 -- > -- > > drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 17 +- > > 2 files changed, 25 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > > b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > > index aec9179..89b3ede 100644 > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h > > @@ -60,6 +60,9 @@ > > /* Top Registers */ > > #define MVPP2_MH_REG(port) (0x5040 + 4 * (port)) > > #define MVPP2_DSA_EXTENDED BIT(5) > > +#define MVPP2_VER_ID_REG 0x50b0 > > +#define MVPP2_VER_PP22 0x10 > > +#define MVPP2_VER_PP23 0x11 > > Looking at the Armada 8040 docs, it seems this register exists on > PPv2.1 as well, and holds the value zero there. > > I wonder whether we should instead read it's value directly into hw_version, > and test against these values, rather than inventing our own verison enum. > > I've also been wondering whether your != MVPP21 comparisons should > instead be >= MVPP22. > > Any thoughts? We cannot access PPv2 register space before enabling clocks(done in mvpp2_probe) , PP21 and PP22/23 have different sets of clocks. So diff between PP21 and PP22/23 should be stored in device tree(in of_device_id), with MVPP22 and MVPP21 stored as .data Maybe we can do it differently, but I prefer to make this change not in the Flow Control patch series. I'm OK with both >= MVPP22 and != MVPP21 options. Regards, Stefan.
RE: [EXT] Re: [PATCH v2 RFC net-next 09/18] net: mvpp2: add FCA RXQ non occupied descriptor threshold
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -1154,6 +1154,9 @@ static void mvpp2_interrupts_mask(void *arg)
> > mvpp2_thread_write(port->priv,
> >mvpp2_cpu_to_thread(port->priv,
> smp_processor_id()),
> >MVPP2_ISR_RX_TX_MASK_REG(port->id), 0);
> > + mvpp2_thread_write(port->priv,
> > + mvpp2_cpu_to_thread(port->priv,
> smp_processor_id()),
> > + MVPP2_ISR_RX_ERR_CAUSE_REG(port->id), 0);
>
> I wonder if this should be refactored:
>
> u32 thread = mvpp2_cpu_to_thread(port->priv,
> smp_processor_id());
>
> mvpp2_thread_write(port->priv, thread,
> MVPP2_ISR_RX_TX_MASK_REG(port->id), 0);
> mvpp2_thread_write(port->priv, thread,
> MVPP2_ISR_RX_ERR_CAUSE_REG(port->id), 0);
>
> to avoid having to recompute mvpp2_cpu_to_thread() for each write?
>
> However, looking deeper...
>
> static void mvpp2_interrupts_mask(void *arg) {
> struct mvpp2_port *port = arg;
> u32 thread;
> int cpu;
>
> cpu = smp_processor_id();
> if (cpu > port->priv->nthreads)
> return
>
> thread = mvpp2_cpu_to_thread(port->priv, cpu);
> ...
>
> and I wonder about that condition - "cpu > port->priv->nthreads". If cpu ==
> port->priv->nthreads, then mvpp2_cpu_to_thread() will return zero, just like
> the cpu=0 case. This leads me to suspect that this comparison off by one.
I can push patch that make it if (cpu => port->priv->nthreads). Or even remove
this if.
Anyway on current Armada platforms we have only 4 CPU's and maximum 9 PPv2
threads(nthreads is min between num_present_cpus and maximum HW PPv2 threads),
so this would be always false.
Regards,
Stefan.
RE: [EXT] Re: [PATCH v2 RFC net-next 13/18] net: mvpp2: add ethtool flow control configuration support
>
> --
> On Sun, Jan 24, 2021 at 01:44:02PM +0200, stef...@marvell.com wrote:
> > @@ -6407,6 +6490,29 @@ static void mvpp2_mac_link_up(struct
> phylink_config *config,
> > val);
> > }
> >
> > + if (tx_pause && port->priv->global_tx_fc) {
> > + port->tx_fc = true;
> > + mvpp2_rxq_enable_fc(port);
> > + if (port->priv->percpu_pools) {
> > + for (i = 0; i < port->nrxqs; i++)
> > + mvpp2_bm_pool_update_fc(port,
> >priv->bm_pools[i], true);
> > + } else {
> > + mvpp2_bm_pool_update_fc(port, port->pool_long,
> true);
> > + mvpp2_bm_pool_update_fc(port, port->pool_short,
> true);
> > + }
> > +
> > + } else if (port->priv->global_tx_fc) {
> > + port->tx_fc = false;
> > + mvpp2_rxq_disable_fc(port);
> > + if (port->priv->percpu_pools) {
> > + for (i = 0; i < port->nrxqs; i++)
> > + mvpp2_bm_pool_update_fc(port,
> >priv->bm_pools[i], false);
> > + } else {
> > + mvpp2_bm_pool_update_fc(port, port->pool_long,
> false);
> > + mvpp2_bm_pool_update_fc(port, port->pool_short,
> false);
> > + }
> > + }
> > +
>
> It seems this can be written more succinctly:
>
> if (port->priv->global_tx_fc) {
> port->tx_fc = tx_pause;
> if (tx_pause)
> mvpp2_rxq_enable_fc(port);
> else
> mvpp2_rxq_disable_fc(port);
> if (port->priv->percpu_pools) {
> for (i = 0; i < port->nrxqs; i++)
> mvpp2_bm_pool_update_fc(port,
> >priv->bm_pools[i],
> tx_pause);
> } else {
> mvpp2_bm_pool_update_fc(port, port->pool_long,
> tx_pause);
> mvpp2_bm_pool_update_fc(port, port->pool_short,
> tx_pause);
> }
> }
>
Ok, I would update.
Thanks,
Stefan.
Re: [PATCH] kvm: Add emulation for movups/movupd
Am 13.01.21 um 00:47 schrieb Jim Mattson: On Wed, Apr 4, 2018 at 10:44 PM Paolo Bonzini wrote: On 04/04/2018 19:35, Stefan Fritsch wrote: On Wednesday, 4 April 2018 19:24:20 CEST Paolo Bonzini wrote: On 04/04/2018 19:10, Konrad Rzeszutek Wilk wrote: Should there be a corresponding test-case? Good point! Stefan, could you write one? Is there infrastructure for such tests? If yes, can you give me a pointer to it? Yes, check out x86/emulator.c in https://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git. There is already a movaps test. Whatever became of this unit test? I don't see it in the kvm-unit-tests repository. Sorry, I did not get around to doing this. And it is unlikely that I will have time to do it in the near future. Cheers, Stefan
RE: [EXT] Re: [PATCH net-next] net: mvpp2: extend mib-fragments name to mib-fragments-err
> > From: Stefan Chulski
> >
> > This patch doesn't change any functionality, but just extend MIB
> > counter register and ethtool-statistic names with "err".
> >
> > The counter MVPP2_MIB_FRAGMENTS_RCVD in fact is Error counter.
> > Extend REG name and appropriated ethtool statistic reg-name with the
> > ERR/err.
>
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -1566,7 +1566,7 @@ static u32 mvpp2_read_index(struct mvpp2
> *priv, u32 index, u32 reg)
> > { MVPP2_MIB_FC_RCVD, "fc_received" },
> > { MVPP2_MIB_RX_FIFO_OVERRUN, "rx_fifo_overrun" },
> > { MVPP2_MIB_UNDERSIZE_RCVD, "undersize_received" },
> > - { MVPP2_MIB_FRAGMENTS_RCVD, "fragments_received" },
> > + { MVPP2_MIB_FRAGMENTS_ERR_RCVD, "fragments_err_received" },
>
> Hi Stefan
>
> I suspect this is now ABI and you cannot change it. You at least need to argue
> why it is not ABI.
>
> Andrew
Hi Andrew,
I not familiar with ABI concept. Does this mean we cannot change, fix or extend
driver ethtool counters?
Thanks,
Stefan.
Re: [PATCH] mmc: sdhci-iproc: Add ACPI bindings for the rpi4
Hi,
Am 11.01.21 um 04:39 schrieb Jeremy Linton:
> Hi,
>
> On 1/9/21 5:07 AM, Stefan Wahren wrote:
>> Hi Jeremy,
>>
>> +add Nicolas
>>
>> Am 08.01.21 um 22:13 schrieb Jeremy Linton:
>>> The rpi4 has a Arasan controller it carries over
>>> from the rpi3, and a newer eMMC2 controller.
>>> Because of a couple "quirks" it seems wiser to bind
>>> these controllers to the same driver that DT is using
>>> on this platform rather than the generic sdhci_acpi
>>> driver with PNP0D40.
>>>
>>> So, we use BCM2847 for the older Arasan and BRCME88C
>>> for the newer eMMC2.
>>>
>>> With this change linux is capable of utilizing the
>>> SD card slot, and the wifi on this platform
>>> with linux.
>>>
>>> Signed-off-by: Jeremy Linton
>>> ---
>>> drivers/mmc/host/sdhci-iproc.c | 14 ++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-iproc.c
>>> b/drivers/mmc/host/sdhci-iproc.c
>>> index c9434b461aab..f79d97b41805 100644
>>> --- a/drivers/mmc/host/sdhci-iproc.c
>>> +++ b/drivers/mmc/host/sdhci-iproc.c
>>> @@ -250,6 +250,14 @@ static const struct sdhci_pltfm_data
>>> sdhci_bcm2835_pltfm_data = {
>>> .ops = _iproc_32only_ops,
>>> };
>>> +static const struct sdhci_pltfm_data sdhci_bcm_arasan_data = {
>>> + .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
>>> + SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
>>> + SDHCI_QUIRK_NO_HISPD_BIT,
>>> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
>>> + .ops = _iproc_32only_ops,
>>> +};
>
> First, thanks for taking a look at this!
>
>
>> Why do we need an almost exact copy of bcm2835_data which works fine for
>> all Raspberry Pi boards?
>
> The short answer to the remainder of this email is that i'm trying to
> continue supporting existing OSs (windows) using the ACPI tables on
> the rpi3/rpi4 while adding rpi4+Linux support.
>
> An even shorter answer is they don't work because ACPI doesn't provide
> the same clock/attributes/etc controls that exist with DT.
>
> So, what happened here is that I got this controller "working" with
> the generic PNP0D40 sdhci_acpi driver. I managed this only by
> controlling the sdhci_caps/masks in the firmware. In theory this
> minimizes the amount of work needed on the other OS which are booting
> on the same ACPI tables (*bsds). They should only need to quirk the
> bcm/arasan specific functionality, rather than some of the quirking
> which change the caps behavior. But because we don't know which if any
> of the older rpi/arasan quirks are still needed the safest solution is
> to use the _iproc driver and just drop the quirk flags known to be
> worked around by the firmware caps override.
okay, thanks for the explanation. I was also confused by bcm_arasan,
because there is already an Arasan specific sdhci driver. But now it's
clear to me.
Could you please add a short comment (above sdhci_bcm_arasan_data) why
we cannot use bcm2835_data?
Btw the subject isn't complete. The patch is also related to the rpi3.
Best regards
Stefan
RE: [EXT] Re: [PATCH RFC net-next 00/19] net: mvpp2: Add TX Flow Control support
> > On Sun, Jan 10, 2021 at 06:55:11PM +, Stefan Chulski wrote: > > > > not connected to the GOP flow control generation mechanism. > > > > To solve this issue Armada has firmware running on CM3 CPU > > > > dedectated for Flow Control support. Firmware monitors Packet > > > > Processor resources and asserts XON/XOFF by writing to Ports Control 0 > Register. > > > > > > What is the minimum firmware version that supports this? > > > > > > > Support were added to firmware about two years ago. > > All releases from 18.09 should has it. > > Can you query the firmware and ask its version? We should keep all this code > disabled if the firmware it too old. > > Andrew This is exactly what " net: mvpp2: add TX FC firmware check " patch do. If handshake of flow control support fail, FC won't be supported. Stefan.
RE: [EXT] Re: [PATCH RFC net-next 00/19] net: mvpp2: Add TX Flow Control support
> > not connected to the GOP flow control generation mechanism. > > To solve this issue Armada has firmware running on CM3 CPU dedectated > > for Flow Control support. Firmware monitors Packet Processor resources > > and asserts XON/XOFF by writing to Ports Control 0 Register. > > What is the minimum firmware version that supports this? > Support were added to firmware about two years ago. All releases from 18.09 should has it. Stefan, Regards.
RE: [EXT] Re: [PATCH RFC net-next 11/19] net: mvpp2: add flow control RXQ and BM pool config callbacks
> Sorry, but that is not really a decision the driver can make. It is part of a > kernel > that _does_ support CPU hotplug, and the online CPUs can be changed today. > > It is likely that every distro out there builds the kernel with CPU hotplug > enabled. > > If changing the online CPUs causes the driver to misbehave, that is a(nother) > bug with the driver. This function doesn't really need to know num_active_cpus, only host ID used by used by shared RX interrupt in single queue mode. Host ID is just register address space used to access PPv2 register space. So I can remove this use of num_active_cpus. Stefan, Regards.
RE: [EXT] Re: [PATCH RFC net-next 14/19] net: mvpp2: add ethtool flow control configuration support
> > @@ -5373,6 +5402,30 @@ static int
> mvpp2_ethtool_set_pause_param(struct net_device *dev,
> > struct ethtool_pauseparam *pause) {
> > struct mvpp2_port *port = netdev_priv(dev);
> > + int i;
> > +
> > + if (pause->tx_pause && port->priv->global_tx_fc) {
> > + port->tx_fc = true;
> > + mvpp2_rxq_enable_fc(port);
> > + if (port->priv->percpu_pools) {
> > + for (i = 0; i < port->nrxqs; i++)
> > + mvpp2_bm_pool_update_fc(port,
> >priv->bm_pools[i], true);
> > + } else {
> > + mvpp2_bm_pool_update_fc(port, port->pool_long,
> true);
> > + mvpp2_bm_pool_update_fc(port, port->pool_short,
> true);
> > + }
> > +
> > + } else if (port->priv->global_tx_fc) {
> > + port->tx_fc = false;
> > + mvpp2_rxq_disable_fc(port);
> > + if (port->priv->percpu_pools) {
> > + for (i = 0; i < port->nrxqs; i++)
> > + mvpp2_bm_pool_update_fc(port,
> >priv->bm_pools[i], false);
> > + } else {
> > + mvpp2_bm_pool_update_fc(port, port->pool_long,
> false);
> > + mvpp2_bm_pool_update_fc(port, port->pool_short,
> false);
> > + }
> > + }
>
> This doesn't look correct to me. This function is only called when ethtool -A
> is
> used to change the flow control settings. This is not the place to be
> configuring flow control, as flow control is negotiated with the link partner.
>
> The final resolved flow control settings are available in
> mvpp2_mac_link_up() via the tx_pause and rx_pause parameters.
I would move this to mvpp2_mac_link_up.
Thanks.
> What also concerns me is whether flow control is supported in the existing
> driver at all, given this patch set. If it isn't supported without the
> firmware's
> help, then we should _not_ be negotiating flow control with the link partner
> unless we actually support it, so the Pause and Asym_Pause bits in
> mvpp2_phylink_validate() should be cleared.
RX FC supported, issue only with TX FC.
Stefan,
Regards.
RE: [EXT] Re: [PATCH RFC net-next 11/19] net: mvpp2: add flow control RXQ and BM pool config callbacks
> >
> > +/* Routine calculate single queue shares address space */ static int
> > +mvpp22_calc_shared_addr_space(struct mvpp2_port *port) {
> > + /* If number of CPU's greater than number of threads, return last
> > +* address space
> > +*/
> > + if (num_active_cpus() >= MVPP2_MAX_THREADS)
> > + return MVPP2_MAX_THREADS - 1;
> > +
> > + return num_active_cpus();
>
> Firstly - this can be written as:
>
> return min(num_active_cpus(), MVPP2_MAX_THREADS - 1);
OK.
> Secondly - what if the number of active CPUs change, for example due to
> hotplug activity. What if we boot with maxcpus=1 and then bring the other
> CPUs online after networking has been started? The number of active CPUs is
> dynamically managed via the scheduler as CPUs are brought online or offline.
>
> > +/* Routine enable flow control for RXQs conditon */ void
> > +mvpp2_rxq_enable_fc(struct mvpp2_port *port)
> ...
> > +/* Routine disable flow control for RXQs conditon */ void
> > +mvpp2_rxq_disable_fc(struct mvpp2_port *port)
>
> Nothing seems to call these in this patch, so on its own, it's not obvious how
> these are being called, and therefore what remedy to suggest for
> num_active_cpus().
I don't think that current driver support CPU hotplug, anyway I can remove
num_active_cpus
and just use shared RX IRQ ID.
Thanks.
.
RE: [EXT] Re: [PATCH RFC net-next 03/19] net: mvpp2: add CM3 SRAM memory map
> > > > + } else {
> > > > + priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> > > > + if (!priv->sram_pool) {
> > > > + dev_warn(>dev, "DT is too old, TX FC
> > > disabled\n");
> > >
> > > I don't see anything in this patch that disables TX flow control,
> > > which means this warning message is misleading.
> >
> > OK, I would change to TX FC not supported.
>
> And you should tell phlylink, so it knows to disable it in autoneg.
>
> Which make me wonder, do we need a fix for stable? Has flow control never
> been support in this device up until these patches get merged?
> It should not be negotiated if it is not supported, which means telling
> phylink.
>
>Andrew
TX FC never were really supported. MAC or PHY can negotiated flow control.
But MAC would never trigger FC frame.
Should I prepare separate patch that disable TX FC till we merge this patches?
Regards,
Stefan.
RE: [EXT] Re: [PATCH RFC net-next 03/19] net: mvpp2: add CM3 SRAM memory map
> > + } else {
> > + priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> > + if (!priv->sram_pool) {
> > + dev_warn(>dev, "DT is too old, TX FC
> disabled\n");
>
> I don't see anything in this patch that disables TX flow control, which means
> this warning message is misleading.
OK, I would change to TX FC not supported.
Stefan.
RE: [EXT] Re: [PATCH RFC net-next 14/19] net: mvpp2: add ethtool flow control configuration support
> > @@ -5373,6 +5402,30 @@ static int
> mvpp2_ethtool_set_pause_param(struct net_device *dev,
> > struct ethtool_pauseparam *pause) {
> > struct mvpp2_port *port = netdev_priv(dev);
> > + int i;
> > +
> > + if (pause->tx_pause && port->priv->global_tx_fc) {
> > + port->tx_fc = true;
> > + mvpp2_rxq_enable_fc(port);
> > + if (port->priv->percpu_pools) {
> > + for (i = 0; i < port->nrxqs; i++)
> > + mvpp2_bm_pool_update_fc(port,
> >priv->bm_pools[i], true);
> > + } else {
> > + mvpp2_bm_pool_update_fc(port, port->pool_long,
> true);
> > + mvpp2_bm_pool_update_fc(port, port->pool_short,
> true);
> > + }
> > +
> > + } else if (port->priv->global_tx_fc) {
> > + port->tx_fc = false;
> > + mvpp2_rxq_disable_fc(port);
> > + if (port->priv->percpu_pools) {
> > + for (i = 0; i < port->nrxqs; i++)
> > + mvpp2_bm_pool_update_fc(port,
> >priv->bm_pools[i], false);
> > + } else {
> > + mvpp2_bm_pool_update_fc(port, port->pool_long,
> false);
> > + mvpp2_bm_pool_update_fc(port, port->pool_short,
> false);
> > + }
> > + }
>
>
> This looks wrong. Flow control is normally the result of auto negotiation.
> Both
> ends need to agree to it. Which is why
> mvpp2_ethtool_set_pause_param() passes the users request onto phylink.
> phylink will handle the autoneg and then ask the MAC to setup flow control
> depending on the result in mvpp2_mac_link_up().
>
> Andrew
Ok, I would move it to mvpp2_mac_link_up.
Stefan,
Thanks.
RE: [EXT] Re: [PATCH RFC net-next 03/19] net: mvpp2: add CM3 SRAM memory map
>
> > > Should there be -EPROBE_DEFER handling in here somewhere? The SRAM
> > > is a device, so it might not of been probed yet?
> >
>
> > No, firmware probed during bootloader boot and we can use SRAM. SRAM
> > memory can be safely used.
>
> A previous patch added:
>
> + CP11X_LABEL(cm3_sram): cm3@22 {
> + compatible = "mmio-sram";
> + reg = <0x22 0x800>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x22 0x800>;
> + };
> +
>
> So it looks like the SRAM is a device, in the linux driver model. And there
> is a
> driver for this, driver/misc/sram.c. How do you know this device has been
> probed before the Ethernet driver?
>
>Andrew
You right, I would add EPROBE_DEFER if of_gen_pool_get return NULL.
Thanks.
RE: [EXT] Re: [PATCH RFC net-next 06/19] net: mvpp2: increase BM pool size to 2048 buffers
> External Email
>
> --
> On Sun, Jan 10, 2021 at 05:30:10PM +0200, stef...@marvell.com wrote:
> > From: Stefan Chulski
> >
> > BM pool size increased to support Firmware Flow Control.
> > Minimum depletion thresholds to support FC is 1024 buffers.
> > BM pool size increased to 2048 to have some 1024 buffers space between
> > depletion thresholds and BM pool size.
> >
> > Signed-off-by: Stefan Chulski
> > ---
> > drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> > index 89b3ede..8dc669d 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> > @@ -851,8 +851,8 @@ enum mvpp22_ptp_packet_format {
> > #define MVPP22_PTP_TIMESTAMPQUEUESELECTBIT(18)
> >
> > /* BM constants */
> > -#define MVPP2_BM_JUMBO_BUF_NUM 512
> > -#define MVPP2_BM_LONG_BUF_NUM 1024
> > +#define MVPP2_BM_JUMBO_BUF_NUM 2048
> > +#define MVPP2_BM_LONG_BUF_NUM 2048
>
> Hi Stefan
>
> Jumbo used to be 1/2 of regular. Do you know why?
>
> It would be nice to have a comment in the commit message about why it is
> O.K. to change the ratio of jumbo to regular frames, and what if anything this
> does for memory requirements.
>
>Andrew
I don't know why it is half(no hardware restrictions for this). I would add to
commit message new memory requirements for buffer allocations.
Thanks.
RE: [EXT] Re: [PATCH RFC net-next 03/19] net: mvpp2: add CM3 SRAM memory map
> -Original Message-
> From: Andrew Lunn
> Sent: Sunday, January 10, 2021 7:05 PM
> To: Stefan Chulski
> Cc: net...@vger.kernel.org; thomas.petazz...@bootlin.com;
> da...@davemloft.net; Nadav Haklai ; Yan Markman
> ; linux-kernel@vger.kernel.org; k...@kernel.org;
> li...@armlinux.org.uk; m...@semihalf.com; rmk+ker...@armlinux.org.uk;
> aten...@kernel.org
> Subject: [EXT] Re: [PATCH RFC net-next 03/19] net: mvpp2: add CM3 SRAM
> memory map
>
> External Email
>
> --
> > +static int mvpp2_get_sram(struct platform_device *pdev,
> > + struct mvpp2 *priv)
> > +{
> > + struct device_node *dn = pdev->dev.of_node;
> > + struct resource *res;
> > +
> > + if (has_acpi_companion(>dev)) {
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> > + if (!res) {
> > + dev_warn(>dev, "ACPI is too old, TX FC
> disabled\n");
> > + return 0;
> > + }
> > + priv->cm3_base = devm_ioremap_resource(>dev,
> res);
> > + if (IS_ERR(priv->cm3_base))
> > + return PTR_ERR(priv->cm3_base);
> > + } else {
> > + priv->sram_pool = of_gen_pool_get(dn, "cm3-mem", 0);
> > + if (!priv->sram_pool) {
> > + dev_warn(>dev, "DT is too old, TX FC
> disabled\n");
> > + return 0;
> > + }
> > + priv->cm3_base = (void __iomem *)gen_pool_alloc(priv-
> >sram_pool,
> > +
> MSS_SRAM_SIZE);
> > + if (!priv->cm3_base)
> > + return -ENOMEM;
>
> Should there be -EPROBE_DEFER handling in here somewhere? The SRAM is a
> device, so it might not of been probed yet?
No, firmware probed during bootloader boot and we can use SRAM. SRAM memory can
be safely used.
Regards.
Re: [PATCH] mmc: sdhci-iproc: Add ACPI bindings for the rpi4
Hi Jeremy,
+add Nicolas
Am 08.01.21 um 22:13 schrieb Jeremy Linton:
> The rpi4 has a Arasan controller it carries over
> from the rpi3, and a newer eMMC2 controller.
> Because of a couple "quirks" it seems wiser to bind
> these controllers to the same driver that DT is using
> on this platform rather than the generic sdhci_acpi
> driver with PNP0D40.
>
> So, we use BCM2847 for the older Arasan and BRCME88C
> for the newer eMMC2.
>
> With this change linux is capable of utilizing the
> SD card slot, and the wifi on this platform
> with linux.
>
> Signed-off-by: Jeremy Linton
> ---
> drivers/mmc/host/sdhci-iproc.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c
> index c9434b461aab..f79d97b41805 100644
> --- a/drivers/mmc/host/sdhci-iproc.c
> +++ b/drivers/mmc/host/sdhci-iproc.c
> @@ -250,6 +250,14 @@ static const struct sdhci_pltfm_data
> sdhci_bcm2835_pltfm_data = {
> .ops = _iproc_32only_ops,
> };
>
> +static const struct sdhci_pltfm_data sdhci_bcm_arasan_data = {
> + .quirks = SDHCI_QUIRK_BROKEN_CARD_DETECTION |
> + SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK |
> + SDHCI_QUIRK_NO_HISPD_BIT,
> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> + .ops = _iproc_32only_ops,
> +};
Why do we need an almost exact copy of bcm2835_data which works fine for
all Raspberry Pi boards?
> +
> static const struct sdhci_iproc_data bcm2835_data = {
> .pdata = _bcm2835_pltfm_data,
> .caps = ((0x1 << SDHCI_MAX_BLOCK_SHIFT)
> @@ -261,6 +269,10 @@ static const struct sdhci_iproc_data bcm2835_data = {
> .mmc_caps = 0x,
> };
>
> +static const struct sdhci_iproc_data bcm_arasan_data = {
> + .pdata = _bcm_arasan_data,
> +};
> +
> static const struct sdhci_ops sdhci_iproc_bcm2711_ops = {
> .read_l = sdhci_iproc_readl,
> .read_w = sdhci_iproc_readw,
> @@ -299,6 +311,8 @@ MODULE_DEVICE_TABLE(of, sdhci_iproc_of_match);
> static const struct acpi_device_id sdhci_iproc_acpi_ids[] = {
> { .id = "BRCM5871", .driver_data = (kernel_ulong_t)_cygnus_data },
> { .id = "BRCM5872", .driver_data = (kernel_ulong_t)_data },
> + { .id = "BCM2847", .driver_data = (kernel_ulong_t)_arasan_data },
Sorry, i don't have deeper knowledge about ACPI, but BCM2837 is the
official naming of the SoC on the RPi 3.
Is this a typo in the id?
> + { .id = "BRCME88C", .driver_data = (kernel_ulong_t)_data },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(acpi, sdhci_iproc_acpi_ids);
Fw: Hiding the vim user in amdgpu_vcn.c (typo in comment)
Hi, Ricardo Mestre wrote: > Stefan Hagen wrote: >> I can totally relate to this one. >> >> Found after a conversation about muscle memory and grepping the >> source tree for ":wq". >> > This issue is present in upstream [0], please take it with them. > > [0] > https://sourcegraph.com/github.com/torvalds/linux@30bb5572ce7a8710fa9ea720b6ecb382791c9800/-/blob/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c#L131 Forwarding to upstream. The patch below is for the Linux source tree. Best Regards, Stefan From e4511830bd58ac1508dde86b440a36e15a92b5cc Mon Sep 17 00:00:00 2001 From: c0dev0id Date: Tue, 5 Jan 2021 13:50:13 +0100 Subject: [PATCH] drm/amdgpu: fix typo in comment --- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c index 4a77c7424dfc..0c9eeb42d518 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c @@ -168,7 +168,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev) /* Bit 20-23, it is encode major and non-zero for new naming convention. * This field is part of version minor and DRM_DISABLED_FLAG in old naming -* convention. Since the l:wq!atest version minor is 0x5B and DRM_DISABLED_FLAG +* convention. Since the latest version minor is 0x5B and DRM_DISABLED_FLAG * is zero in old naming convention, this field is always zero so far. * These four bits are used to tell which naming convention is present. */ -- 2.30.0
Re: [PATCH v2] vhost/vsock: add IOTLB API support
On Wed, Dec 23, 2020 at 03:36:38PM +0100, Stefano Garzarella wrote: > This patch enables the IOTLB API support for vhost-vsock devices, > allowing the userspace to emulate an IOMMU for the guest. > > These changes were made following vhost-net, in details this patch: > - exposes VIRTIO_F_ACCESS_PLATFORM feature and inits the iotlb > device if the feature is acked > - implements VHOST_GET_BACKEND_FEATURES and > VHOST_SET_BACKEND_FEATURES ioctls > - calls vq_meta_prefetch() before vq processing to prefetch vq > metadata address in IOTLB > - provides .read_iter, .write_iter, and .poll callbacks for the > chardev; they are used by the userspace to exchange IOTLB messages > > This patch was tested specifying "intel_iommu=strict" in the guest > kernel command line. I used QEMU with a patch applied [1] to fix a > simple issue (that patch was merged in QEMU v5.2.0): > $ qemu -M q35,accel=kvm,kernel-irqchip=split \ >-drive file=fedora.qcow2,format=qcow2,if=virtio \ >-device intel-iommu,intremap=on,device-iotlb=on \ >-device vhost-vsock-pci,guest-cid=3,iommu_platform=on,ats=on > > [1] https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg09077.html > > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Stefano Garzarella > --- > > The patch is the same of v1, but I re-tested it with: > - QEMU v5.2.0-551-ga05f8ecd88 > - Linux 5.9.15 (host) > - Linux 5.9.15 and 5.10.0 (guest) > Now, enabling 'ats' it works well, there are just a few simple changes. > > v1: https://www.spinics.net/lists/kernel/msg3716022.html > v2: > - updated commit message about QEMU version and string used to test > - rebased on mst/vhost branch > > Thanks, > Stefano > --- > drivers/vhost/vsock.c | 68 +-- > 1 file changed, 65 insertions(+), 3 deletions(-) Acked-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [PATCH 06/21] vdpa: introduce virtqueue groups
On Wed, Dec 16, 2020 at 02:48:03PM +0800, Jason Wang wrote: > This patch introduces virtqueue groups to vDPA device. The virtqueue > group is the minimal set of virtqueues that must share an address > space. And the adddress space identifier could only be attached to > a specific virtqueue group. > > A new mandated bus operation is introduced to get the virtqueue group > ID for a specific virtqueue. > > All the vDPA device drivers were converted to simply support a single > virtqueue group. > > Signed-off-by: Jason Wang > --- > drivers/vdpa/ifcvf/ifcvf_main.c | 9 - > drivers/vdpa/mlx5/net/mlx5_vnet.c | 8 +++- > drivers/vdpa/vdpa.c | 4 +++- > drivers/vdpa/vdpa_sim/vdpa_sim.c | 11 ++- > include/linux/vdpa.h | 12 +--- > 5 files changed, 37 insertions(+), 7 deletions(-) Maybe consider calling it iotlb_group or iommu_group so the purpose of the group is clear? signature.asc Description: PGP signature
[PATCH] MAINTAINERS: Include bcm2835 subsequents into search
Change the bcm2835 maintainer info in order to handle subsequent SoCs. After this get_maintainers.pl provides the proper maintainers for irqchip-bcm2836. Signed-off-by: Stefan Wahren --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 546aa66..ecfe78f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3413,7 +3413,7 @@ F: Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml F: drivers/pci/controller/pcie-brcmstb.c F: drivers/staging/vc04_services N: bcm2711 -N: bcm2835 +N: bcm283* BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITECTURE M: Florian Fainelli -- 2.7.4
Re: [PATCH] tty: serial: amba-pl011: added RS485 support v2
char_transfer_time =
> + (unsigned long) div_u64(
> + mul_u32_u32(
> + (u32)transfer_bit_count,
> + (u32)NSEC_PER_SEC),
> + (u32)baud);
> +#endif
> +
> spin_lock_irqsave(>lock, flags);
>
> /*
> @@ -2020,6 +2425,11 @@ pl011_set_termios(struct uart_port *port, struct
> ktermios *termios,
> old_cr = pl011_read(uap, REG_CR);
> pl011_write(0, uap, REG_CR);
>
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> + /* Update send_char_time in locked context */
> + uap->send_char_time = char_transfer_time;
> +#endif
> +
> if (termios->c_cflag & CRTSCTS) {
> if (old_cr & UART011_CR_RTS)
> old_cr |= UART011_CR_RTSEN;
> @@ -2091,6 +2501,7 @@ static const char *pl011_type(struct uart_port *port)
> {
> struct uart_amba_port *uap =
> container_of(port, struct uart_amba_port, port);
> +
Please avoid unrelated whitespace changes
> return uap->port.type == PORT_AMBA ? uap->type : NULL;
> }
>
> @@ -2122,6 +2533,47 @@ static void pl011_config_port(struct uart_port *port,
> int flags)
> }
> }
>
> +/*
> + * Configure RS485
> + * Locking: called with port lock held and IRQs disabled
> + */
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +static int pl011_config_rs485(struct uart_port *port,
> + struct serial_rs485 *rs485)
> +{
> + bool was_disabled;
> + struct uart_amba_port *uap =
> + container_of(port, struct uart_amba_port, port);
> +
> + was_disabled = !(port->rs485.flags & SER_RS485_ENABLED);
> +
> + port->rs485.flags = rs485->flags;
> + port->rs485.delay_rts_after_send = rs485->delay_rts_after_send;
> + port->rs485.delay_rts_before_send = rs485->delay_rts_before_send;
> +
> + if (port->rs485.flags & SER_RS485_ENABLED) {
> + unsigned int cr;
> +
> + hrtimer_try_to_cancel(&(uap->rs485_delay_timer));
> + hrtimer_try_to_cancel(&(uap->rs485_tx_empty_poll_timer));
> +
> + /* If RS485 is enabled, disable auto RTS */
> + cr = pl011_read(uap, REG_CR);
> + cr &= ~UART011_CR_RTSEN;
> + pl011_write(cr, uap, REG_CR);
> +
> + uap->rs485_current_status = rs485_receiving;
> + RS485_SET_RTS_SIGNAL(uap,
> + port->rs485.flags
> + & SER_RS485_RTS_AFTER_SEND);
> + } else {
> + RS485_SET_RTS_SIGNAL(uap, true);
> + }
> +
> + return 0;
> +}
> +#endif
> +
> /*
> * verify the new serial_struct (for TIOCSSERIAL).
> */
> @@ -2647,6 +3099,12 @@ static int pl011_probe(struct amba_device *dev, const
> struct amba_id *id)
> uap->port.irq = dev->irq[0];
> uap->port.ops = _pl011_pops;
>
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> + uap->port.rs485_config = _config_rs485;
> + uap->port.rs485.flags = 0; /* RS485 is not enabled by default */
> + dev_info(>dev, "Software switching for RS485 enabled\n");
> +#endif
> +
> snprintf(uap->type, sizeof(uap->type), "PL011 rev%u", amba_rev(dev));
>
> ret = pl011_setup_port(>dev, uap, >res, portnr);
> @@ -2819,10 +3277,15 @@ static struct amba_driver pl011_driver = {
>
> static int __init pl011_init(void)
> {
> +#ifndef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> printk(KERN_INFO "Serial: AMBA PL011 UART driver\n");
> +#else
> + printk(KERN_INFO "Serial: AMBA PL011 UART driver with soft RS485
> support\n");
> +#endif
I think one log message about RS485 support is enough, so please drop
the new dev_info above. An alternative solution for this ifdef could be
the usage of IS_ENABLED()
Thanks
Stefan
>
> if (platform_driver_register(_sbsa_uart_platform_driver))
> pr_warn("could not register SBSA UART platform driver\n");
> +
> return amba_driver_register(_driver);
> }
>
> @@ -2832,6 +3295,11 @@ static void __exit pl011_exit(void)
> amba_driver_unregister(_driver);
> }
>
> +#ifdef CONFIG_SERIAL_AMBA_PL011_SOFT_RS485
> +#undef RS485_SET_RTS_SIGNAL
> +#undef RS485_TX_FIFO_EMPTY
> +#endif
> +
> /*
> * While this can be a module, if builtin it's most likely the console
> * So let's leave module_exit but move module_init to an earlier place
Re: [PATCH] tpm: ibmvtpm: fix error return code in tpm_ibmvtpm_probe()
On 11/25/20 10:35 PM, Jarkko Sakkinen wrote:
On Tue, 2020-11-24 at 21:52 +0800, Wang Hai wrote:
Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.
Fixes: d8d74ea3c002 ("tpm: ibmvtpm: Wait for buffer to be set before
proceeding")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
Provide a reasoning for -ETIMEOUT in the commit message.
/Jarkko
Was this patch ever applied? I don't seem to find the infradead git tree ...
RE: [EXT] Re: [PATCH net-next] net: mvpp2: prs: improve ipv4 parse flow
> > -- > On Thu, 17 Dec 2020 18:07:58 +0200 stef...@marvell.com wrote: > > From: Stefan Chulski > > > > Patch didn't fix any issue, just improve parse flow and align ipv4 > > parse flow with ipv6 parse flow. > > > > Currently ipv4 kenguru parser first check IP protocol(TCP/UDP) and > > then destination IP address. > > Patch introduce reverse ipv4 parse, first destination IP address > > parsed and only then IP protocol. > > This would allow extend capability for packet L4 parsing and align > > ipv4 parsing flow with ipv6. > > > > Suggested-by: Liron Himi > > Signed-off-by: Stefan Chulski > > This one will need to wait until after the merge window > > -- > > # Form letter - net-next is closed > > We have already sent the networking pull request for 5.11 and therefore net- > next is closed for new drivers, features, code refactoring and optimizations. > We are currently accepting bug fixes only. > > Please repost when net-next reopens after 5.11-rc1 is cut. OK, Thanks. > Look out for the announcement on the mailing list or check: > https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_- > 7Edavem_net- > 2Dnext.html=DwICAg=nKjWec2b6R0mOyPaz7xtfQ=DDQ3dKwkTIxKAl > 6_Bs7GMx4zhJArrXKN2mDMOXGh7lg=2CcDqbEJMvxpx15rGBe2og6oh1eZ > hVee8xvK-mjfd0E=r1d6bSIPQmjwJqe- > mkU_s5wyqHOU82D18G6SkVuUg5A= > > RFC patches sent for review only are obviously welcome at any time. If I post RFC patches for review only, should I add some prefix or tag for this? And if all reviewers OK with change(or no comments at all), should I repost this patch again after net-next opened? Thanks, Stefan.
RE: [EXT] Re: [PATCH net v3] net: mvpp2: Fix GoP port 3 Networking Complex Control configurations
> External Email > > -- > On Thu, 17 Dec 2020 14:37:28 +0200 stef...@marvell.com wrote: > > From: Stefan Chulski > > > > During GoP port 2 Networking Complex Control mode of operation > > configurations, also GoP port 3 mode of operation was wrongly set. > > Patch removes these configurations. > > GENCONF_CTRL0_PORTX naming also fixed. > > Testing the stable backport it looks like this addition change will be > problematic. Not to mention it goes against the "fixes should be minimal" > rule. > > Could you please send just a one liner which removes the offending ORing in > of the bad bit? > > We can do the rename soon after in net-next, the trees are merged pretty > much every week so it won't be a long wait. I would repost with single line change. Regards, Stefan.
RE: [EXT] Re: [PATCH net] net: mvpp2: Add TCAM entry to drop flow control pause frames
> Quoting stef...@marvell.com (2020-12-17 18:45:06)
> > From: Stefan Chulski
> >
> > Issue:
> > Flow control frame used to pause GoP(MAC) was delivered to the CPU and
> > created a load on the CPU. Since XOFF/XON frames are used only by MAC,
> > these frames should be dropped inside MAC.
> >
> > Fix:
> > According to 802.3-2012 - IEEE Standard for Ethernet pause frame has
> > unique destination MAC address 01-80-C2-00-00-01.
> > Add TCAM parser entry to track and drop pause frames by destination MAC.
> >
> > Fixes: db9d7d36eecc ("net: mvpp2: Split the PPv2 driver to a dedicated
> > directory")
>
> Same here, you should go further in the git history.
>
> Also, was that introduced when the TCAM support landed in (overriding its
> default configuration?)? Or is that the behaviour since the beginning? I'm
> asking because while this could very be a fix, it could also fall in the
> improvements category.
I believe this behavior since the beginning of the driver.
I would repost this with fixes tag with patch that introduced this driver.
Regards.
RE: [EXT] Re: [PATCH net] net: mvpp2: prs: fix PPPoE with ipv6 packet parse
> External Email
>
> ------
> Hi Stefan,
>
> Quoting stef...@marvell.com (2020-12-17 18:23:11)
> > From: Stefan Chulski
> >
> > Current PPPoE+IPv6 entry is jumping to 'next-hdr'
> > field and not to 'DIP' field as done for IPv4.
> >
> > Fixes: db9d7d36eecc ("net: mvpp2: Split the PPv2 driver to a dedicated
> > directory")
>
> That's not the commit introducing the issue. You can use `git log --follow`
> to go
> further back (or directly pointing to the old mvpp2.c file).
>
> Thanks!
> Antoine
Ok. Probably this issue exist since mvpp2.c driver were pushed to mainline.
Thanks,
Stefan.
RE: [EXT] Re: [PATCH net v2 2/2] net: mvpp2: disable force link UP during port init procedure
> On Thu, Dec 17, 2020 at 12:00:49PM +0100, Marcin Wojtas wrote: > > Hi Stefan, > > > > czw., 17 gru 2020 o 10:42 napisał(a): > > > > > > From: Stefan Chulski > > > > > > Force link UP can be enabled by bootloader during tftpboot and > > > breaks NFS support. > > > Force link UP disabled during port init procedure. > > > > > > Signed-off-by: Stefan Chulski > > > --- > > > > What are the updates against v1? Please note them in this place for > > individual patches and list all in the cover letter (in case sending a > > group of patches). > > It seems the only reason this has been resent is because it's > (incorrectly) part of a series that involved a change to patch 1 (adding the > Fixes: tag). > > As this is a stand-alone fix, it shouldn't be part of a series unless there > really is > some kind of dependency with the other patch(es) of that series. I would repost this two patches separately. Regards, Stefan.
RE: [EXT] Re: [PATCH net 1/2] net: mvpp2: Fix GoP port 3 Networking Complex Control configurations
> -Original Message- > From: Jakub Kicinski > Sent: Thursday, December 17, 2020 2:42 AM > To: Stefan Chulski > Cc: net...@vger.kernel.org; thomas.petazz...@bootlin.com; > da...@davemloft.net; Nadav Haklai ; Yan Markman > ; linux-kernel@vger.kernel.org; > li...@armlinux.org.uk; m...@semihalf.com; and...@lunn.ch; > rmk+ker...@armlinux.org.uk > Subject: [EXT] Re: [PATCH net 1/2] net: mvpp2: Fix GoP port 3 Networking > Complex Control configurations > > External Email > > -- > On Tue, 15 Dec 2020 15:32:12 +0200 stef...@marvell.com wrote: > > From: Stefan Chulski > > > > During GoP port 2 Networking Complex Control mode of operation > > configurations, also GoP port 3 mode of operation was wrongly set mode. > > Patch removes these configurations. > > GENCONF_CTRL0_PORTX naming also fixed. > > Can we get a Fixes tag? Reposting with Fixes tag. Stefan.
[PATCH] x86/kgdb: Allow removal of early BPs
The problem is that breakpoints that are set early (e.g. via kgdbwait)
cannot be deleted after boot completed (to be precise after mark_rodata_ro
ran).
When setting a breakpoint early there are executable pages that are
writable so the copy_to_kernel_nofault call in kgdb_arch_set_breakpoint
succeeds and the breakpoint is saved as type BP_BREAKPOINT.
Later in the boot write access to these pages is restricted. So when
removing the breakpoint the copy_to_kernel_nofault call in
kgdb_arch_remove_breakpoint is destined to fail and the breakpoint removal
fails. So after copy_to_kernel_nofault failed try to text_poke_kgdb which
can work around nonwriteability.
One thing to consider when doing this is that code can go away during boot
(e.g. .init.text). Previously kgdb_arch_remove_breakpoint handled this case
gracefully by just having copy_to_kernel_nofault fail but if one then calls
text_poke_kgdb the system dies due to the BUG_ON we moved out of
__text_poke. To avoid this __text_poke now returns an error in case of a
nonpresent code page and the error is handled at call site.
Checkpatch complains about two uses of BUG_ON but the new code should not
trigger BUG_ON in cases where the old didn't.
Co-developed-by: Lorena Kretzschmar
Signed-off-by: Lorena Kretzschmar
Signed-off-by: Stefan Saecherl
---
arch/x86/kernel/alternative.c | 16 +++
arch/x86/kernel/kgdb.c| 54 ---
2 files changed, 48 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 2400ad62f330..0f145d837885 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -878,11 +878,9 @@ static void *__text_poke(void *addr, const void *opcode,
size_t len)
if (cross_page_boundary)
pages[1] = virt_to_page(addr + PAGE_SIZE);
}
- /*
-* If something went wrong, crash and burn since recovery paths are not
-* implemented.
-*/
- BUG_ON(!pages[0] || (cross_page_boundary && !pages[1]));
+
+ if (!pages[0] || (cross_page_boundary && !pages[1]))
+ return ERR_PTR(-EFAULT);
/*
* Map the page without the global bit, as TLB flushing is done with
@@ -976,7 +974,13 @@ void *text_poke(void *addr, const void *opcode, size_t len)
{
lockdep_assert_held(_mutex);
- return __text_poke(addr, opcode, len);
+ addr = __text_poke(addr, opcode, len);
+ /*
+* If something went wrong, crash and burn since recovery paths are not
+* implemented.
+*/
+ BUG_ON(IS_ERR(addr));
+ return addr;
}
/**
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index ff7878df96b4..e98c9c43db7c 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -731,6 +731,7 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long
ip)
int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
{
int err;
+ void *addr;
bpt->type = BP_BREAKPOINT;
err = copy_from_kernel_nofault(bpt->saved_instr, (char *)bpt->bpt_addr,
@@ -747,8 +748,14 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
*/
if (mutex_is_locked(_mutex))
return -EBUSY;
- text_poke_kgdb((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
- BREAK_INSTR_SIZE);
+
+ addr = text_poke_kgdb((void *)bpt->bpt_addr,
arch_kgdb_ops.gdb_bpt_instr,
+ BREAK_INSTR_SIZE);
+ /* This should never trigger because the above call to
copy_from_kernel_nofault
+* already succeeded.
+*/
+ BUG_ON(IS_ERR(addr));
+
bpt->type = BP_POKE_BREAKPOINT;
return 0;
@@ -756,21 +763,36 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
{
- if (bpt->type != BP_POKE_BREAKPOINT)
- goto knl_write;
- /*
-* It is safe to call text_poke_kgdb() because normal kernel execution
-* is stopped on all cores, so long as the text_mutex is not locked.
-*/
- if (mutex_is_locked(_mutex))
- goto knl_write;
- text_poke_kgdb((void *)bpt->bpt_addr, bpt->saved_instr,
- BREAK_INSTR_SIZE);
- return 0;
+ void *addr;
+ int err;
-knl_write:
- return copy_to_kernel_nofault((char *)bpt->bpt_addr,
- (char *)bpt->saved_instr, BREAK_INSTR_SIZE);
+ if (bpt->type == BP_POKE_BREAKPOINT) {
+ if (mutex_is_locked(_mutex)) {
+ err = copy_to_kernel_nofault((char *)bpt->bpt_addr,
+ (char
*)bpt->saved_instr,
+ BREAK_INSTR_SIZE);
+ } else {
+ /*
+
[PATCH v3 2/5] arm64: dts: meson: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
registers. This fixes an issue seen on ODROID-C2 where the Ethernet
link doesn't come up when using ip link set down/up:
[ 6630.714855] meson8b-dwmac c941.ethernet eth0: Link is Down
[ 6630.785775] meson8b-dwmac c941.ethernet eth0: PHY [stmmac-0:00] driver
[RTL8211F Gigabit Ethernet] (irq=36)
[ 6630.893071] meson8b-dwmac c941.ethernet: Failed to reset the dma
[ 6630.893800] meson8b-dwmac c941.ethernet eth0: stmmac_hw_setup: DMA
engine initialization failed
[ 6630.902835] meson8b-dwmac c941.ethernet eth0: stmmac_open: Hw setup
failed
Fixes: f29cabf240ed ("arm64: dts: meson: use the generic Ethernet PHY reset
GPIO bindings")
Reviewed-by: Martin Blumenstingl
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts| 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts| 2 +-
9 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
index 7be3e354093b..de27beafe9db 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
@@ -165,7 +165,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 70fcfb7b0683..50de1d01e565 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -200,7 +200,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index 222ee8069cfa..9b0b81f191f1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -126,7 +126,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
index ad812854a107..a350fee1264d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
@@ -147,7 +147,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
index b08c4537f260..b2ab05c22090 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
@@ -82,7 +82,7 @@ external_phy: ethernet-phy@0 {
/* External PHY reset is shared with internal PHY Led signal */
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dt
[PATCH v3 5/5] arm64: dts: meson: g12b: w400: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.
Fixes: 2cd2310fca4c ("arm64: dts: meson-g12b-ugoos-am6: add initial
device-tree")
Reviewed-by: Martin Blumenstingl
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
index 2802ddbb83ac..feb088504740 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
@@ -264,7 +264,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <_intc>;
--
2.29.2
[PATCH v3 4/5] arm64: dts: meson: g12a: x96-max: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
registers. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.
Fixes: ed5e8f689154 ("arm64: dts: meson: g12a: x96-max: fix the Ethernet PHY
reset line")
Reviewed-by: Martin Blumenstingl
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
index 1b07c8c06eac..463a72d6bb7c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -340,7 +340,7 @@ external_phy: ethernet-phy@0 {
eee-broken-1000t;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <_intc>;
--
2.29.2
[PATCH v3 1/5] arm64: dts: meson: g12b: odroid-n2: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
registers. This fixes an issue where the Ethernet link doesn't come up
when using ip link set down/up:
[ 29.360965] meson8b-dwmac ff3f.ethernet eth0: Link is Down
[ 34.569012] meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver
[RTL8211F Gigabit Ethernet] (irq=31)
[ 34.676732] meson8b-dwmac ff3f.ethernet: Failed to reset the dma
[ 34.678874] meson8b-dwmac ff3f.ethernet eth0: stmmac_hw_setup: DMA
engine initialization failed
[ 34.687850] meson8b-dwmac ff3f.ethernet eth0: stmmac_open: Hw setup
failed
Fixes: 658e4129bb81 ("arm64: dts: meson: g12b: odroid-n2: add the Ethernet PHY
reset line")
Reviewed-by: Martin Blumenstingl
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
index 6982632ae646..39a09661c5f6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
@@ -413,7 +413,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <_intc>;
--
2.29.2
[PATCH v3 3/5] ARM: dts: meson: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
registers. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.
Fixes: a2c6e82e5341 ("ARM: dts: meson: switch to the generic Ethernet PHY reset
bindings")
Reviewed-by: Martin Blumenstingl
Tested-by: Martin Blumenstingl # on
Odroid-C1+
Signed-off-by: Stefan Agner
---
arch/arm/boot/dts/meson8b-odroidc1.dts| 2 +-
arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts
b/arch/arm/boot/dts/meson8b-odroidc1.dts
index 0c26467de4d0..5963566dbcc9 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -224,7 +224,7 @@ eth_phy: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOH_4 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
index cc498191ddd1..8f4eb1ed4581 100644
--- a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
+++ b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
@@ -81,7 +81,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOH_4 GPIO_ACTIVE_LOW>;
};
};
--
2.29.2
Re: [PATCH v2 3/5] ARM: dts: meson: fix PHY deassert timing requirements
On 2020-12-05 14:04, Martin Blumenstingl wrote:
> Hi Stefan,
>
> On Tue, Dec 1, 2020 at 2:21 PM Stefan Agner wrote:
>>
>> According to the datasheet (Rev. 1.9) the RTL8211F requires at least
>> 72ms "for internal circuits settling time" before accessing the PHY
>> egisters. On similar boards with the same PHY this fixes an issue where
> there's a typo here: it should be "registers"
> this is the same for the other four patches also
Whoops, will send v3 shortly.
>
>> Ethernet link would not come up when using ip link set down/up.
> I have never experienced that myself but gotten a few reports about this.
> thank you very much for coming up with info from the datasheet!
>
> the following stmmac patch [0] has been added recently which may - or
> may not - have any impact also.
Thanks for the hint, wasn't aware of that.
--
Stefan
>
>> Fixes: a2c6e82e5341 ("ARM: dts: meson: switch to the generic Ethernet PHY
>> reset bindings")
>> Signed-off-by: Stefan Agner
> with above typo fixed:
> Reviewed-by: Martin Blumenstingl
> and also:
> Tested-by: Martin Blumenstingl #
> on Odroid-C1+
>
>
> Best regards,
> Martin
>
>
> [0]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/stmicro/stmmac?id=56311a315da7ebc668dbcc2f1c99689cc10796c4
Re: [PATCH] vhost scsi: fix error return code in vhost_scsi_set_endpoint()
On Fri, Dec 04, 2020 at 04:43:30PM +0800, Zhang Changzhong wrote:
> Fix to return a negative error code from the error handling
> case instead of 0, as done elsewhere in this function.
>
> Fixes: 25b98b64e284 ("vhost scsi: alloc cmds per vq instead of session")
> Reported-by: Hulk Robot
> Signed-off-by: Zhang Changzhong
> ---
> drivers/vhost/scsi.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Acked-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
Re: [PATCH] vfio iommu type1: Bypass the vma permission check in vfio_pin_pages_remote()
On Thu, Dec 03, 2020 at 05:01:32PM +0100, David Hildenbrand wrote: > The real question is: do we even *need* DMA from vfio devices to > virtio-fs regions? If not (do guests rely on it? what does the spec > state?), just don't care about vfio at all and don't map anything. Can DMA to/from the virtio-fs DAX Window happen? Yes, the guest could do it although it's rare. Is it a great idea to combine VFIO and virtio-fs DAX? Maybe not. It involves pinning host page cache pages O_o. Stefan signature.asc Description: PGP signature
Re: [PATCH v4 3/3] edac: Supporting AST2400 and AST2600 edac driver
> Adding AST2400 and AST2600 edac driver support.
>
> Signed-off-by: Troy Lee
Reviewed-by: Stefan Schaeckeler
> ---
drivers/edac/Kconfig | 6 +++---
> drivers/edac/aspeed_edac.c | 7 +--
> 2 files changed, 8 insertions(+), 5 deletions(-)
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 7a47680d6f07..c410331e8ee8 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -515,10 +515,10 @@ config EDAC_QCOM
> health, you should probably say 'Y' here.
>
> config EDAC_ASPEED
> - tristate "Aspeed AST 2500 SoC"
> - depends on MACH_ASPEED_G5
> + tristate "Aspeed AST BMC SoC"
> + depends on ARCH_ASPEED
> help
> - Support for error detection and correction on the Aspeed AST 2500 SoC.
> + Support for error detection and correction on the Aspeed AST BMC SoC.
>
> First, ECC must be configured in the bootloader. Then, this driver
> will expose error counters via the EDAC kernel framework.
> diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
> index fde809efc520..a46da56d6d54 100644
> --- a/drivers/edac/aspeed_edac.c
> +++ b/drivers/edac/aspeed_edac.c
> @@ -239,7 +239,7 @@ static int init_csrows(struct mem_ctl_info *mci)
> int rc;
>
> /* retrieve info about physical memory from device tree */
> - np = of_find_node_by_path("/memory");
> + np = of_find_node_by_name(NULL, "memory");
> if (!np) {
> dev_err(mci->pdev, "dt: missing /memory node\n");
> return -ENODEV;
> @@ -375,10 +375,13 @@ static int aspeed_remove(struct platform_device *pdev)
>
>
> static const struct of_device_id aspeed_of_match[] = {
> + { .compatible = "aspeed,ast2400-sdram-edac" },
> { .compatible = "aspeed,ast2500-sdram-edac" },
> + { .compatible = "aspeed,ast2600-sdram-edac" },
> {},
> };
>
> +MODULE_DEVICE_TABLE(of, aspeed_of_match);
>
> static struct platform_driver aspeed_driver = {
> .driver = {
> @@ -392,5 +395,5 @@ module_platform_driver(aspeed_driver);
>
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Stefan Schaeckeler ");
> -MODULE_DESCRIPTION("Aspeed AST2500 EDAC driver");
> +MODULE_DESCRIPTION("Aspeed BMC SoC EDAC driver");
> MODULE_VERSION("1.0");
> --
> 2.17.1
Re: [PATCH 3/3] edac: Supporting AST2400 and AST2600 edac driver
Hello Troy, > Hi Stefan, > > The driver was ported from latest ASPEED BSP, so I only test with ECC-on/off > from u-boot and check if driver runs correctly. I noticed now most changes are these "exports". As you removed them a later revision, the patch looks now lean and clean. I'll give you my Reviewed-by tag after you addressed Andrew's last comment. > The test doc you provided is very nice and detailed, I'll try to reproduce > the > injection test in v2 patch. It does not harm to redo the testing. That is time-consuming and with your current, now trivial changes, it's not really necessary. Stefan
[PATCH] drivers/hv: remove obsolete TODO and fix misleading typo in comment
Removes an obsolete TODO in the VMBus module and fixes a misleading typo in the comment for the macro MAX_NUM_CHANNELS, where two digits have been twisted. Signed-off-by: Stefan Eschenbacher Co-developed-by: Max Stolze Signed-off-by: Max Stolze --- drivers/hv/hyperv_vmbus.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 02f3e8988836..9416e09ebd58 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -187,14 +187,13 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, u64 *requestid, bool raw); /* - * The Maximum number of channels (16348) is determined by the size of the + * The Maximum number of channels (16384) is determined by the size of the * interrupt page, which is HV_HYP_PAGE_SIZE. 1/2 of HV_HYP_PAGE_SIZE is to * send endpoint interrupts, and the other is to receive endpoint interrupts. */ #define MAX_NUM_CHANNELS ((HV_HYP_PAGE_SIZE >> 1) << 3) /* The value here must be in multiple of 32 */ -/* TODO: Need to make this configurable */ #define MAX_NUM_CHANNELS_SUPPORTED 256 #define MAX_CHANNEL_RELIDS \ -- 2.20.1
[PATCH 1/3] drivers/hv: make max_num_channels_supported configurable
To make the number of supported channels configurable, this patch
introduces uint max_num_channels_supported as module parameter.
Also macro MAX_NUM_CHANNELS_SUPPORTED_DEFAULT is introduced with
value 256, which is the currently used macro value.
MAX_NUM_CHANNELS_SUPPORTED was found and replaced in two locations.
During module initialization sanity checks are applied which will result
in EINVAL or ERANGE if the given value is no multiple of 32 or larger than
MAX_NUM_CHANNELS.
Signed-off-by: Stefan Eschenbacher
Co-developed-by: Max Stolze
Signed-off-by: Max Stolze
---
drivers/hv/hyperv_vmbus.h | 6 +++---
drivers/hv/vmbus_drv.c| 20 +++-
2 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 02f3e8988836..edeee1c0497d 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -194,11 +194,11 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
#define MAX_NUM_CHANNELS ((HV_HYP_PAGE_SIZE >> 1) << 3)
/* The value here must be in multiple of 32 */
-/* TODO: Need to make this configurable */
-#define MAX_NUM_CHANNELS_SUPPORTED 256
+#define MAX_NUM_CHANNELS_SUPPORTED_DEFAULT 256
+extern uint max_num_channels_supported;
#define MAX_CHANNEL_RELIDS \
- max(MAX_NUM_CHANNELS_SUPPORTED, HV_EVENT_FLAGS_COUNT)
+ max((ulong) max_num_channels_supported, (ulong) HV_EVENT_FLAGS_COUNT)
enum vmbus_connect_state {
DISCONNECTED,
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 502f8cd95f6d..8f1c8a606b4a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -48,6 +48,11 @@ static int hyperv_cpuhp_online;
static void *hv_panic_page;
+uint max_num_channels_supported = MAX_NUM_CHANNELS_SUPPORTED_DEFAULT;
+module_param(max_num_channels_supported, uint, 0);
+MODULE_PARM_DESC(max_num_channels_supported,
+ "Number of supported vmbus channels. Must be a multiple of 32 and equal
to or less than 16348");
+
/* Values parsed from ACPI DSDT */
static int vmbus_irq;
int vmbus_interrupt;
@@ -1220,7 +1225,7 @@ static void vmbus_chan_sched(struct hv_per_cpu_context
*hv_cpu)
u32 maxbits, relid;
if (vmbus_proto_version < VERSION_WIN8) {
- maxbits = MAX_NUM_CHANNELS_SUPPORTED;
+ maxbits = max_num_channels_supported;
recv_int_page = vmbus_connection.recv_int_page;
} else {
/*
@@ -2620,6 +2625,17 @@ static int __init hv_acpi_init(void)
if (!hv_is_hyperv_initialized())
return -ENODEV;
+ // Check if max_num_channels_supported is a multiple of 32 and smaller
MAX_NUM_CHANNELS
+ if (max_num_channels_supported % 32 != 0) {
+ pr_err("max_num_channels_supported is %u, but must be a
multiple of 32\n",
+ max_num_channels_supported);
+ return -EINVAL;
+ } else if (max_num_channels_supported > MAX_NUM_CHANNELS) {
+ pr_err("max_num_channels_supported is %u which exceeds maximum
of %lu\n",
+ max_num_channels_supported, MAX_NUM_CHANNELS);
+ return -ERANGE;
+ }
+
init_completion(_event);
/*
@@ -2641,6 +2657,8 @@ static int __init hv_acpi_init(void)
if (ret)
goto cleanup;
+ pr_info("Supporting up to %u channels\n", max_num_channels_supported);
+
hv_setup_kexec_handler(hv_kexec_handler);
hv_setup_crash_handler(hv_crash_handler);
--
2.20.1
[PATCH 3/3] drivers/hv: add default number of vmbus channels to Kconfig
The default number of vmbus channels (macro MAX_NUM_CHANNELS_SUPPORTED_DEFAULT) is made configurable through the new Kconfig option HYPERV_VMBUS_DEFAULT_CHANNELS. Signed-off-by: Stefan Eschenbacher Co-developed-by: Max Stolze Signed-off-by: Max Stolze --- drivers/hv/Kconfig| 13 + drivers/hv/hyperv_vmbus.h | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig index 79e5356a737a..40bee5b05ccd 100644 --- a/drivers/hv/Kconfig +++ b/drivers/hv/Kconfig @@ -26,4 +26,17 @@ config HYPERV_BALLOON help Select this option to enable Hyper-V Balloon driver. +config HYPERV_VMBUS_DEFAULT_CHANNELS + int "Default number of VMBus channels (as multiple of 32)" + range 0 512 + default 8 + depends on HYPERV + help + Specify the default number of VMBus channels here as nth multiple of + 32. The value must be equal to or less than 512. + The default is 8 meaning the number of channels is 8 * 32 = 256. + A different value can be set when loading the hv_vmbus module by + setting the parameter max_num_channels_supported directly to the + number of channels and not as a multiple of 32. + endmenu diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 52dcfa1c17ef..47af2c76ce57 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -194,7 +194,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, #define MAX_NUM_CHANNELS ((HV_HYP_PAGE_SIZE >> 1) << 3) /* The value here must be in multiple of 32 */ -#define MAX_NUM_CHANNELS_SUPPORTED_DEFAULT 256 +#define MAX_NUM_CHANNELS_SUPPORTED_DEFAULT (CONFIG_HYPERV_VMBUS_DEFAULT_CHANNELS * 32) extern uint max_num_channels_supported; #define MAX_CHANNEL_RELIDS \ -- 2.20.1
[PATCH 2/3] drivers/hv: fix misleading typo in comment
Fixes a misleading typo in the comment for the macro MAX_NUM_CHANNELS, where two digits have been twisted. Signed-off-by: Stefan Eschenbacher Co-developed-by: Max Stolze Signed-off-by: Max Stolze --- drivers/hv/hyperv_vmbus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index edeee1c0497d..52dcfa1c17ef 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -187,7 +187,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel, u64 *requestid, bool raw); /* - * The Maximum number of channels (16348) is determined by the size of the + * The Maximum number of channels (16384) is determined by the size of the * interrupt page, which is HV_HYP_PAGE_SIZE. 1/2 of HV_HYP_PAGE_SIZE is to * send endpoint interrupts, and the other is to receive endpoint interrupts. */ -- 2.20.1
[PATCH 0/3] drivers/hv: make max_num_channels_supported configurable
According to the TODO comment in hyperv_vmbus.h the value in macro MAX_NUM_CHANNELS_SUPPORTED should be configurable. The first patch accomplishes that by introducting uint max_num_channels_supported as module parameter. Also macro MAX_NUM_CHANNELS_SUPPORTED_DEFAULT is introduced with value 256, which is the currently used macro value. MAX_NUM_CHANNELS_SUPPORTED was found and replaced in two locations. During module initialization sanity checks are applied which will result in EINVAL or ERANGE if the given value is no multiple of 32 or larger than MAX_NUM_CHANNELS. While testing, we found a misleading typo in the comment for the macro MAX_NUM_CHANNELS which is fixed by the second patch. The third patch makes the added default macro configurable by introduction and use of Kconfig parameter HYPERV_VMBUS_DEFAULT_CHANNELS. Default value remains at 256. Two notes on these patches: 1) With above patches it is valid to configure max_num_channels_supported and MAX_NUM_CHANNELS_SUPPORTED_DEFAULT as 0. We simply don't know if that is a valid value. Doing so while testing still left us with a working Debian VM in Hyper-V on Windows 10. 2) To set the Kconfig parameter the user currently has to divide the desired default number of channels by 32 and enter the result of that calculation. This way both known constraints we got from the comments in the code are enforced. It feels a bit unintuitive though. We haven't found Kconfig options to ensure that the value is a multiple of 32. So if you'd like us to fix that we'd be happy for some input on how to settle it with Kconfig. Signed-off-by: Stefan Eschenbacher Co-developed-by: Max Stolze Signed-off-by: Max Stolze Stefan Eschenbacher (3): drivers/hv: make max_num_channels_supported configurable drivers/hv: fix misleading typo in comment drivers/hv: add default number of vmbus channels to Kconfig drivers/hv/Kconfig| 13 + drivers/hv/hyperv_vmbus.h | 8 drivers/hv/vmbus_drv.c| 20 +++- 3 files changed, 36 insertions(+), 5 deletions(-) -- 2.20.1
Re: [PATCH] vfio iommu type1: Bypass the vma permission check in vfio_pin_pages_remote()
On Wed, Dec 02, 2020 at 10:45:11AM -0500, Peter Xu wrote: > On Wed, Dec 02, 2020 at 02:33:56PM +0000, Stefan Hajnoczi wrote: > > On Wed, Nov 25, 2020 at 10:57:11AM -0500, Peter Xu wrote: > > > On Wed, Nov 25, 2020 at 01:05:25AM +, Justin He wrote: > > > > > I'd appreciate if you could explain why vfio needs to dma map some > > > > > PROT_NONE > > > > > > > > Virtiofs will map a PROT_NONE cache window region firstly, then remap > > > > the sub > > > > region of that cache window with read or write permission. I guess this > > > > might > > > > be an security concern. Just CC virtiofs expert Stefan to answer it > > > > more accurately. > > > > > > Yep. Since my previous sentence was cut off, I'll rephrase: I was > > > thinking > > > whether qemu can do vfio maps only until it remaps the PROT_NONE regions > > > into > > > PROT_READ|PROT_WRITE ones, rather than trying to map dma pages upon > > > PROT_NONE. > > > > Userspace processes sometimes use PROT_NONE to reserve virtual address > > space. That way future mmap(NULL, ...) calls will not accidentally > > allocate an address from the reserved range. > > > > virtio-fs needs to do this because the DAX window mappings change at > > runtime. Initially the entire DAX window is just reserved using > > PROT_NONE. When it's time to mmap a portion of a file into the DAX > > window an mmap(fixed_addr, ...) call will be made. > > Yes I can understand the rational on why the region is reserved. However IMHO > the real question is why such reservation behavior should affect qemu memory > layout, and even further to VFIO mappings. > > Note that PROT_NONE should likely mean that there's no backing page at all in > this case. Since vfio will pin all the pages before mapping the DMAs, it also > means that it's at least inefficient, because when we try to map all the > PROT_NONE pages we'll try to fault in every single page of it, even if they > may > not ever be used. > > So I still think this patch is not doing the right thing. Instead we should > somehow teach qemu that the virtiofs memory region should only be the size of > enabled regions (with PROT_READ|PROT_WRITE), rather than the whole reserved > PROT_NONE region. virtio-fs was not implemented with IOMMUs in mind. The idea is just to install a kvm.ko memory region that exposes the DAX window. Perhaps we need to treat the DAX window like an IOMMU? That way the virtio-fs code can send map/unmap notifications and hw/vfio/ can propagate them to the host kernel. Stefan signature.asc Description: PGP signature
Re: [PATCH net-next v1 1/3] vm_sockets: Include flag field in the vsock address data structure
On Tue, Dec 01, 2020 at 05:25:03PM +0200, Andra Paraschiv wrote: > vsock enables communication between virtual machines and the host they > are running on. With the multi transport support (guest->host and > host->guest), nested VMs can also use vsock channels for communication. > > In addition to this, by default, all the vsock packets are forwarded to > the host, if no host->guest transport is loaded. This behavior can be > implicitly used for enabling vsock communication between sibling VMs. > > Add a flag field in the vsock address data structure that can be used to > explicitly mark the vsock connection as being targeted for a certain > type of communication. This way, can distinguish between nested VMs and > sibling VMs use cases and can also setup them at the same time. Till > now, could either have nested VMs or sibling VMs at a time using the > vsock communication stack. > > Use the already available "svm_reserved1" field and mark it as a flag > field instead. This flag can be set when initializing the vsock address > variable used for the connect() call. > > Signed-off-by: Andra Paraschiv > --- > include/uapi/linux/vm_sockets.h | 18 +- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h > index fd0ed7221645d..58da5a91413ac 100644 > --- a/include/uapi/linux/vm_sockets.h > +++ b/include/uapi/linux/vm_sockets.h > @@ -114,6 +114,22 @@ > > #define VMADDR_CID_HOST 2 > > +/* This sockaddr_vm flag value covers the current default use case: > + * local vsock communication between guest and host and nested VMs setup. > + * In addition to this, implicitly, the vsock packets are forwarded to the > host > + * if no host->guest vsock transport is set. > + */ > +#define VMADDR_FLAG_DEFAULT_COMMUNICATION0x > + > +/* Set this flag value in the sockaddr_vm corresponding field if the vsock > + * channel needs to be setup between two sibling VMs running on the same > host. > + * This way can explicitly distinguish between vsock channels created for > nested > + * VMs (or local communication between guest and host) and the ones created > for > + * sibling VMs. And vsock channels for multiple use cases (nested / sibling > VMs) > + * can be setup at the same time. > + */ > +#define VMADDR_FLAG_SIBLING_VMS_COMMUNICATION0x0001 vsock has the h2g and g2h concept. It would be more general to call this flag VMADDR_FLAG_G2H or less cryptically VMADDR_FLAG_TO_HOST. That way it just tells the driver in which direction to send packets without implying that sibling communication is possible (it's not allowed by default on any transport). I don't have a strong opinion on this but wanted to suggest the idea. Stefan signature.asc Description: PGP signature
Re: [PATCH] vfio iommu type1: Bypass the vma permission check in vfio_pin_pages_remote()
On Wed, Nov 25, 2020 at 10:57:11AM -0500, Peter Xu wrote: > On Wed, Nov 25, 2020 at 01:05:25AM +, Justin He wrote: > > > I'd appreciate if you could explain why vfio needs to dma map some > > > PROT_NONE > > > > Virtiofs will map a PROT_NONE cache window region firstly, then remap the > > sub > > region of that cache window with read or write permission. I guess this > > might > > be an security concern. Just CC virtiofs expert Stefan to answer it more > > accurately. > > Yep. Since my previous sentence was cut off, I'll rephrase: I was thinking > whether qemu can do vfio maps only until it remaps the PROT_NONE regions into > PROT_READ|PROT_WRITE ones, rather than trying to map dma pages upon PROT_NONE. Userspace processes sometimes use PROT_NONE to reserve virtual address space. That way future mmap(NULL, ...) calls will not accidentally allocate an address from the reserved range. virtio-fs needs to do this because the DAX window mappings change at runtime. Initially the entire DAX window is just reserved using PROT_NONE. When it's time to mmap a portion of a file into the DAX window an mmap(fixed_addr, ...) call will be made. Stefan signature.asc Description: PGP signature
[PATCH v2 4/5] arm64: dts: meson: g12a: x96-max: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.
Fixes: ed5e8f689154 ("arm64: dts: meson: g12a: x96-max: fix the Ethernet PHY
reset line")
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
index 1b07c8c06eac..463a72d6bb7c 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-g12a-x96-max.dts
@@ -340,7 +340,7 @@ external_phy: ethernet-phy@0 {
eee-broken-1000t;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <_intc>;
--
2.29.2
[PATCH v2 5/5] arm64: dts: meson: g12b: w400: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.
Fixes: 2cd2310fca4c ("arm64: dts: meson-g12b-ugoos-am6: add initial
device-tree")
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
index 2802ddbb83ac..feb088504740 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-w400.dtsi
@@ -264,7 +264,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <_intc>;
--
2.29.2
[PATCH v2 3/5] ARM: dts: meson: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. On similar boards with the same PHY this fixes an issue where
Ethernet link would not come up when using ip link set down/up.
Fixes: a2c6e82e5341 ("ARM: dts: meson: switch to the generic Ethernet PHY reset
bindings")
Signed-off-by: Stefan Agner
---
arch/arm/boot/dts/meson8b-odroidc1.dts| 2 +-
arch/arm/boot/dts/meson8m2-mxiii-plus.dts | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/meson8b-odroidc1.dts
b/arch/arm/boot/dts/meson8b-odroidc1.dts
index 0c26467de4d0..5963566dbcc9 100644
--- a/arch/arm/boot/dts/meson8b-odroidc1.dts
+++ b/arch/arm/boot/dts/meson8b-odroidc1.dts
@@ -224,7 +224,7 @@ eth_phy: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOH_4 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
index cc498191ddd1..8f4eb1ed4581 100644
--- a/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
+++ b/arch/arm/boot/dts/meson8m2-mxiii-plus.dts
@@ -81,7 +81,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOH_4 GPIO_ACTIVE_LOW>;
};
};
--
2.29.2
[PATCH v2 1/5] arm64: dts: meson: g12b: odroid-n2: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. This fixes an issue where the Ethernet link doesn't come up
when using ip link set down/up:
[ 29.360965] meson8b-dwmac ff3f.ethernet eth0: Link is Down
[ 34.569012] meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver
[RTL8211F Gigabit Ethernet] (irq=31)
[ 34.676732] meson8b-dwmac ff3f.ethernet: Failed to reset the dma
[ 34.678874] meson8b-dwmac ff3f.ethernet eth0: stmmac_hw_setup: DMA
engine initialization failed
[ 34.687850] meson8b-dwmac ff3f.ethernet eth0: stmmac_open: Hw setup
failed
Fixes: 658e4129bb81 ("arm64: dts: meson: g12b: odroid-n2: add the Ethernet PHY
reset line")
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
index 6982632ae646..39a09661c5f6 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
@@ -413,7 +413,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <_intc>;
--
2.29.2
[PATCH v2 2/5] arm64: dts: meson: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.9) the RTL8211F requires at least
72ms "for internal circuits settling time" before accessing the PHY
egisters. This fixes an issue seen on ODROID-C2 where the Ethernet
link doesn't come up when using ip link set down/up:
[ 6630.714855] meson8b-dwmac c941.ethernet eth0: Link is Down
[ 6630.785775] meson8b-dwmac c941.ethernet eth0: PHY [stmmac-0:00] driver
[RTL8211F Gigabit Ethernet] (irq=36)
[ 6630.893071] meson8b-dwmac c941.ethernet: Failed to reset the dma
[ 6630.893800] meson8b-dwmac c941.ethernet eth0: stmmac_hw_setup: DMA
engine initialization failed
[ 6630.902835] meson8b-dwmac c941.ethernet eth0: stmmac_open: Hw setup
failed
Fixes: f29cabf240ed ("arm64: dts: meson: use the generic Ethernet PHY reset
GPIO bindings")
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-nexbox-a1.dts | 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-q200.dts| 2 +-
arch/arm64/boot/dts/amlogic/meson-gxm-rbox-pro.dts| 2 +-
9 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
index 7be3e354093b..de27beafe9db 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-nanopi-k2.dts
@@ -165,7 +165,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
index 70fcfb7b0683..50de1d01e565 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-odroidc2.dts
@@ -200,7 +200,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
index 222ee8069cfa..9b0b81f191f1 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-vega-s95.dtsi
@@ -126,7 +126,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
index ad812854a107..a350fee1264d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-wetek.dtsi
@@ -147,7 +147,7 @@ eth_phy0: ethernet-phy@0 {
reg = <0>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
index b08c4537f260..b2ab05c22090 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d-p230.dts
@@ -82,7 +82,7 @@ external_phy: ethernet-phy@0 {
/* External PHY reset is shared with internal PHY Led signal */
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <8>;
reset-gpios = < GPIOZ_14 GPIO_ACTIVE_LOW>;
interrupt-parent = <_intc>;
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
index bff8ec2c
Re: [PATCH] arm64: dts: meson: fix PHY deassert timing requirements
On 2020-12-01 09:31, Jerome Brunet wrote:
> On Tue 01 Dec 2020 at 01:25, Stefan Agner wrote:
>
>> According to the datasheet (Rev. 1.4, page 30) the RTL8211F requires
>> at least 50ms "for internal circuits settling time" before accessing
>> the PHY registers. This fixes an issue where the Ethernet link doesn't
>> come up when using ip link set down/up:
>> [ 29.360965] meson8b-dwmac ff3f.ethernet eth0: Link is Down
>> [ 34.569012] meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver
>> [RTL8211F Gigabit Ethernet] (irq=31)
>> [ 34.676732] meson8b-dwmac ff3f.ethernet: Failed to reset the dma
>> [ 34.678874] meson8b-dwmac ff3f.ethernet eth0: stmmac_hw_setup: DMA
>> engine initialization failed
>> [ 34.687850] meson8b-dwmac ff3f.ethernet eth0: stmmac_open: Hw setup
>> failed
>>
>> Fixes: 658e4129bb81 ("arm64: dts: meson: g12b: odroid-n2: add the Ethernet
>> PHY reset line")
>> Signed-off-by: Stefan Agner
>> ---
>> arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
>> b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
>> index 6982632ae646..a5652caacb27 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
>> @@ -413,7 +413,7 @@ external_phy: ethernet-phy@0 {
>> max-speed = <1000>;
>>
>> reset-assert-us = <1>;
>> -reset-deassert-us = <3>;
>> +reset-deassert-us = <5>;
>> reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW |
>> GPIO_OPEN_DRAIN)>;
>>
>> interrupt-parent = <_intc>;
>
> Thanks for sharing this is Stefan,
> The title of your patch should probably be modified to show that it
> addresses the odroid n2 only, as it stands.
Yes make sense. Hm, are there other boards with RTL8211F? From the
comments in the DT it seems several other boards use the same PHY. Some
however do not have any reset timing data at all currently it seems.
>
> I have checked the RTL8211F doc I have, v1.9, and this one mention
> "72ms at least - not including the 1.0V supply rise time" before
> accessing the PHY registers :/ ... so 80ms maybe ?
Uh interesting, so it seems they increased it over documentation
revisions. Yeah agreed 80ms is the safer value then.
FWIW, I did test it with 50ms in a continuous loop for an hour or so
without seeing any failure, but that was room temperature only.
--
Stefan
[PATCH] arm64: dts: meson: fix PHY deassert timing requirements
According to the datasheet (Rev. 1.4, page 30) the RTL8211F requires
at least 50ms "for internal circuits settling time" before accessing
the PHY registers. This fixes an issue where the Ethernet link doesn't
come up when using ip link set down/up:
[ 29.360965] meson8b-dwmac ff3f.ethernet eth0: Link is Down
[ 34.569012] meson8b-dwmac ff3f.ethernet eth0: PHY [0.0:00] driver
[RTL8211F Gigabit Ethernet] (irq=31)
[ 34.676732] meson8b-dwmac ff3f.ethernet: Failed to reset the dma
[ 34.678874] meson8b-dwmac ff3f.ethernet eth0: stmmac_hw_setup: DMA
engine initialization failed
[ 34.687850] meson8b-dwmac ff3f.ethernet eth0: stmmac_open: Hw setup
failed
Fixes: 658e4129bb81 ("arm64: dts: meson: g12b: odroid-n2: add the Ethernet PHY
reset line")
Signed-off-by: Stefan Agner
---
arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
index 6982632ae646..a5652caacb27 100644
--- a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2.dtsi
@@ -413,7 +413,7 @@ external_phy: ethernet-phy@0 {
max-speed = <1000>;
reset-assert-us = <1>;
- reset-deassert-us = <3>;
+ reset-deassert-us = <5>;
reset-gpios = < GPIOZ_15 (GPIO_ACTIVE_LOW |
GPIO_OPEN_DRAIN)>;
interrupt-parent = <_intc>;
--
2.29.2
Re: [PATCH 3/3] edac: Supporting AST2400 and AST2600 edac driver
Hello Troy, > Adding AST2400 and AST2600 edac driver support. > > Signed-off-by: Troy Lee > --- > drivers/edac/Kconfig | 6 +- > drivers/edac/aspeed_edac.c | 114 + > 2 files changed, 94 insertions(+), 26 deletions(-) Uh, there are quite some non-trivial changes. I'll have a look over the coming weekend. Testing an edac driver comes with challenges. Did you test your code? If so, how? That's how I was testing my original edac 2500 driver http://students.engr.scu.edu/~sschaeck/misc/aspeed-edac.html Stefan
Re: boot interrupt quirk (also in 4.19.y) breaks serial ports (was: [PATCH v2 0/2] pci: Add boot interrupt quirk mechanism for Xeon chipsets)
Hi tglx, On 11/27/20 12:45 AM, Thomas Gleixner wrote: > Stefan, > > On Wed, Nov 25 2020 at 14:41, Stefan Bühler wrote: >> On 11/25/20 12:54 PM, Thomas Gleixner wrote: >>> On Wed, Sep 16 2020 at 12:12, Stefan Bühler wrote: >>> Can you please provide the output of: >>> >>> for ID in 05:00.0 06:00.0 06:00.1 06:01.0 06:01.1; do lspci -s $ID -vvv; >>> done >> >> 05:00.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI Express-to-PCI >> Bridge (rev aa) (prog-if 00 [Normal decode]) >> ... >> Capabilities: > > Can you please run this as root so the Capabilities are accessible? My bad, sorry. I did intend to run it as root, but should have checked the output. Again see attached file. While we're at it: the EEPROM for the PEX8112 is: 5a 03 3c 00 10 00 00 00 00 00 00 00 b5 10 12 81 |Z.<.| 0010 64 00 20 00 00 00 00 01 04 00 01 00 0c 10 00 fe |d. .| 0020 fe 03 20 10 f0 10 00 00 00 10 33 00 00 00 70 00 |.. ...3...p.| 0030 00 00 11 00 48 00 00 00 00 00 34 00 50 00 00 00 |H.4.P...| 0040 04 00 55 66 77 88 |..Ufw.| 0046 (This is what the firmware tool provided to me writes, although I think the cards usually came pre-flashed with it. They gave me the tool because on some cards the second function on OX16PCI954 was sometimes uninitialized, came up with device id 0x9511 "8-bit bus" (PCI_DEVICE_ID_OXSEMI_16PCI95N) and the kernel tries to treat it as UART too.) I think some time ago I found a PDF to decode this here: https://www.broadcom.com/products/pcie-switches-bridges/pcie-bridges/pex8112#documentation But the broadcom site is completely broken right now (at least for me; there own search for "PEX 8112" links it, but then it says "not found"). Anyway, back then I decoded this to: - `0x5A 0x03`: Magic Header, contains register and shared memory settings - `0x003C` = 60 bytes for configs (10 registers): - `@0x0010`: `0x` -- BAR0: Locate anywhere in 32-bit - `@0x`: `0x811210B5` -- Vendor `10B5`, Device `8112` (default) - `@0x0064`: `0x0020` -- Device Capability: Enable "Support 8-bit Tag" field - `@0x0100`: `0x00010004` -- Power Budget Enhanced Capability Header (default) - `@0x100C`: `0x03FEFE00` -- PCI Control: - PCI-To-PCI Express Retry Count set to 0xFE (default: `0x80`) - PCI Express-to-PCI Retry Count set to 0xFE (default: `0x00`) - `@0x1020`: `0x10F0` -- GPIO Control - GPIO[1-3] Output enable (GPIO[0] is Output enabled by default) - GPIO Diagnostic Select: `10b` (default: `01b`) - `@0x1000`: `0x0033` -- Device Initialization (default) - `@0x0070`: `0x0011` -- Link control: default - `@0x0048`: `0x` -- Device-Specific Control (default 0) - `@0x0034`: `0x0050` -- PCI Capability pointer `0x50` (default: `0x40`) - Skips (disables) Power Management Capability - Remaining: MSI and PCI Express - `0x0004` bytes for shared memory: - `0x55`, `0x66`, `0x77`, `0x88` TLDR: the most notable part probably being "disabling Power Management Capability" by the EEPROM. cheers, Stefan 05:00.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI Express-to-PCI Bridge (rev aa) (prog-if 00 [Normal decode]) Physical Slot: 1 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+ Address: Data: Capabilities: [60] Express (v1) PCI-Express to PCI/PCI-X Bridge, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag+ AttnBtn- AttnInd- PwrInd- RBE- SlotPowerLimit 26.000W DevCtl: Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd- ExtTag+ PhantFunc- AuxPwr- NoSnoop- BrConfRtry- MaxPayload 128 bytes, MaxReadReq 512 bytes DevSta: CorrErr- UncorrErr+ FatalErr- UnsuppReq+ AuxPwr- TransPend- LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <1us, L1 <16us ClockPM- Surprise- LLActRep- BwNot- ASPMOptComp- LnkCtl: ASPM Disabled; RCB 64 bytes Disabled- CommClk- ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt- LnkSta: Speed 2.5GT/s, Width x1, TrErr- Train- SlotClk- DLActive- BWMgmt- ABWMgmt- Capabilities: [100 v1] Power Budgeting 06:00.0 Serial controller: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Uart) (prog-if 06 [16950])
Re: boot interrupt quirk (also in 4.19.y) breaks serial ports (was: [PATCH v2 0/2] pci: Add boot interrupt quirk mechanism for Xeon chipsets)
Hi tglx, On 11/25/20 12:54 PM, Thomas Gleixner wrote: > Stefan, > > On Wed, Sep 16 2020 at 12:12, Stefan Bühler wrote: > > sorry for the delay. This fell through the cracks. > >> this quirk breaks our serial ports PCIe card (i.e. we don't see any >> output from the connected devices; no idea whether anything we send >> reaches them): >> >> 05:00.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI Express-to-PCI >> Bridge (rev aa) >> 06:00.0 Serial controller: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 >> UART) function 0 (Uart) >> 06:00.1 Bridge: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) >> function 0 (Disabled) >> 06:01.0 Serial controller: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 >> UART) function 0 (Uart) >> 06:01.1 Bridge: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) >> function 0 (Disabled) > > Can you please provide the output of: > > for ID in 05:00.0 06:00.0 06:00.1 06:01.0 06:01.1; do lspci -s $ID -vvv; done > See attachment. Also I boot the affected systems now with "pci=noioapicquirk", which "solves" the issue too (instead of patching the kernel). cheers, Stefan 05:00.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI Express-to-PCI Bridge (rev aa) (prog-if 00 [Normal decode]) Physical Slot: 1 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- Capabilities: 06:00.0 Serial controller: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Uart) (prog-if 06 [16950]) Subsystem: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Uart) Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Kernel driver in use: serial 06:00.1 Bridge: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Disabled) Subsystem: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Disabled) Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- 06:01.0 Serial controller: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Uart) (prog-if 06 [16950]) Subsystem: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Uart) Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR- Kernel driver in use: serial 06:01.1 Bridge: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Disabled) Subsystem: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Disabled) Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- SERR-
Re: [PATCH] tpm: ibmvtpm: fix error return code in tpm_ibmvtpm_probe()
On 11/24/20 8:52 AM, Wang Hai wrote:
Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.
Fixes: d8d74ea3c002 ("tpm: ibmvtpm: Wait for buffer to be set before
proceeding")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
---
drivers/char/tpm/tpm_ibmvtpm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 994385bf37c0..813eb2cac0ce 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -687,6 +687,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
ibmvtpm->rtce_buf != NULL,
HZ)) {
dev_err(dev, "CRQ response timed out\n");
+ rc = -ETIMEDOUT;
goto init_irq_cleanup;
}
Reviewed-by: Stefan Berger
RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
> -Original Message- > From: Russell King - ARM Linux admin > Sent: Monday, November 23, 2020 7:30 PM > To: Stefan Chulski > Cc: net...@vger.kernel.org; thomas.petazz...@bootlin.com; > da...@davemloft.net; Nadav Haklai ; Yan Markman > ; linux-kernel@vger.kernel.org; k...@kernel.org; > m...@semihalf.com; and...@lunn.ch > Subject: Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports > only > > On Mon, Nov 23, 2020 at 04:03:00PM +, Stefan Chulski wrote: > > I agree with you. We can use "max-speed" for better FIFO allocations. > > I plan to upstream more fixes from the "Marvell" devel branch then I can > prepare this patch. > > So you OK with this patch and then follow-on improvement? > > Yes - but I would like to see the commit description say that this results in > no > change the situation where all three ports are in use. Ok, I would repost patch. Regards.
RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
> -Original Message- > From: Russell King - ARM Linux admin > Sent: Monday, November 23, 2020 5:52 PM > To: Stefan Chulski > Cc: net...@vger.kernel.org; thomas.petazz...@bootlin.com; > da...@davemloft.net; Nadav Haklai ; Yan Markman > ; linux-kernel@vger.kernel.org; k...@kernel.org; > m...@semihalf.com; and...@lunn.ch > Subject: Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports > only > > On Mon, Nov 23, 2020 at 03:44:05PM +, Stefan Chulski wrote: > > Yes, but this allocation exists also in current code. > > From HW point of view(MAC and PPv2) maximum supported speed in CP110: > > port 0 - 10G, port 1 - 2.5G, port 2 - 2.5G. > > in CP115: port 0 - 10G, port 1 - 5G, port 2 - 2.5G. > > > > So this allocation looks correct at least for CP115. > > Problem that we cannot reallocate FIFO during runtime, after specific speed > negotiation. > > We could do much better. DT has a "max-speed" property for ethernet > controllers. If we have that property, then I think we should use that to > determine the initialisation time FIFO allocation. > > As I say, on Macchiatobin, the allocations we end up with are just crazy when > you consider the port speeds that the hardware supports. > Maybe that should be done as a follow-on patch - but I think it needs to be > done. I agree with you. We can use "max-speed" for better FIFO allocations. I plan to upstream more fixes from the "Marvell" devel branch then I can prepare this patch. So you OK with this patch and then follow-on improvement? Regards.
RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
> > > On Mon, Nov 23, 2020 at 04:52:40PM +0200, stef...@marvell.com wrote: > > > > From: Stefan Chulski > > > > > > > > Tx/Rx FIFO is a HW resource limited by total size, but shared by > > > > all ports of same CP110 and impacting port-performance. > > > > Do not divide the FIFO for ports which are not enabled in DTS, so > > > > active ports could have more FIFO. > > > > > > > > The active port mapping should be done in probe before FIFO-init. > > > > > > It would be nice to know what the effect is from this - is it a > > > small or large boost in performance? > > > > I didn't saw any significant improvement with LINUX bridge or > > forwarding, but this reduced PPv2 overruns drops, reduced CRC sent errors > with DPDK user space application. > > So this improved zero loss throughput. Probably with XDP we would see a > similar effect. > > > > > What is the effect when the ports on a CP110 are configured for 10G, > > > 1G, and 2.5G in that order, as is the case on the Macchiatobin board? > > > > Macchiatobin has two CP's. CP1 has 3 ports, so the distribution of FIFO > > would > be the same as before. > > On CP0 which has a single port, all FIFO would be allocated for 10G port. > > Your code allocates for CP1: > > 32K to port 0 (the 10G port on Macchiatobin) 8K to port 1 (the 1G dedicated > ethernet port on Macchiatobin) 4K to port 2 (the 1G/2.5G SFP port on > Macchiatobin) > > I'm questioning that allocation for port 1 and 2. Yes, but this allocation exists also in current code. >From HW point of view(MAC and PPv2) maximum supported speed in CP110: port 0 - 10G, port 1 - 2.5G, port 2 - 2.5G. in CP115: port 0 - 10G, port 1 - 5G, port 2 - 2.5G. So this allocation looks correct at least for CP115. Problem that we cannot reallocate FIFO during runtime, after specific speed negotiation. Regards.
RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
> -Original Message- > From: Russell King - ARM Linux admin > Sent: Monday, November 23, 2020 5:11 PM > To: Stefan Chulski > Cc: net...@vger.kernel.org; thomas.petazz...@bootlin.com; > da...@davemloft.net; Nadav Haklai ; Yan Markman > ; linux-kernel@vger.kernel.org; k...@kernel.org; > m...@semihalf.com; antoine.ten...@bootlin.com; and...@lunn.ch > Subject: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports > only > > External Email > > -- > Hi, > > On Mon, Nov 23, 2020 at 04:52:40PM +0200, stef...@marvell.com wrote: > > From: Stefan Chulski > > > > Tx/Rx FIFO is a HW resource limited by total size, but shared by all > > ports of same CP110 and impacting port-performance. > > Do not divide the FIFO for ports which are not enabled in DTS, so > > active ports could have more FIFO. > > > > The active port mapping should be done in probe before FIFO-init. > > It would be nice to know what the effect is from this - is it a small or large > boost in performance? I didn't saw any significant improvement with LINUX bridge or forwarding, but this reduced PPv2 overruns drops, reduced CRC sent errors with DPDK user space application. So this improved zero loss throughput. Probably with XDP we would see a similar effect. > What is the effect when the ports on a CP110 are configured for 10G, 1G, and > 2.5G in that order, as is the case on the Macchiatobin board? Macchiatobin has two CP's. CP1 has 3 ports, so the distribution of FIFO would be the same as before. On CP0 which has a single port, all FIFO would be allocated for 10G port. Regards.
Re: [PATCH net] vsock: forward all packets to the host when no H2G is registered
On Thu, Nov 12, 2020 at 02:38:37PM +0100, Stefano Garzarella wrote:
> Before commit c0cfa2d8a788 ("vsock: add multi-transports support"),
> if a G2H transport was loaded (e.g. virtio transport), every packets
> was forwarded to the host, regardless of the destination CID.
> The H2G transports implemented until then (vhost-vsock, VMCI) always
> responded with an error, if the destination CID was not
> VMADDR_CID_HOST.
>
> From that commit, we are using the remote CID to decide which
> transport to use, so packets with remote CID > VMADDR_CID_HOST(2)
> are sent only through H2G transport. If no H2G is available, packets
> are discarded directly in the guest.
>
> Some use cases (e.g. Nitro Enclaves [1]) rely on the old behaviour
> to implement sibling VMs communication, so we restore the old
> behavior when no H2G is registered.
> It will be up to the host to discard packets if the destination is
> not the right one. As it was already implemented before adding
> multi-transport support.
>
> Tested with nested QEMU/KVM by me and Nitro Enclaves by Andra.
>
> [1] Documentation/virt/ne_overview.rst
>
> Cc: Jorgen Hansen
> Cc: Dexuan Cui
> Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> Reported-by: Andra Paraschiv
> Tested-by: Andra Paraschiv
> Signed-off-by: Stefano Garzarella
> ---
> net/vmw_vsock/af_vsock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Stefan Hajnoczi
signature.asc
Description: PGP signature
RE: [PATCH] net: mvpp2: divide fifo for dts-active ports only
> -Original Message-
> From: stef...@marvell.com
> Sent: Wednesday, November 18, 2020 8:21 PM
> To: net...@vger.kernel.org
> Cc: thomas.petazz...@bootlin.com; da...@davemloft.net; Nadav Haklai
> ; Yan Markman ; linux-
> ker...@vger.kernel.org; Stefan Chulski
> Subject: [PATCH] net: mvpp2: divide fifo for dts-active ports only
>
> From: Stefan Chulski
>
> Tx/Rx FIFO is a HW resource limited by total size, but shared by all ports of
> same CP110 and impacting port-performance.
> Do not divide the FIFO for ports which are not enabled in DTS, so active ports
> could have more FIFO.
>
> The active port mapping should be done in probe before FIFO-init.
>
> Signed-off-by: Stefan Chulski
> ---
> drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 23 +++--
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 129
> +---
> 2 files changed, 108 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> index 8347758..6bd7e40 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
> @@ -695,6 +695,9 @@
> /* Maximum number of supported ports */
> #define MVPP2_MAX_PORTS 4
>
> +/* Loopback port index */
> +#define MVPP2_LOOPBACK_PORT_INDEX3
> +
> /* Maximum number of TXQs used by single port */
> #define MVPP2_MAX_TXQ8
>
> @@ -729,22 +732,21 @@
> #define MVPP2_TX_DESC_ALIGN (MVPP2_DESC_ALIGNED_SIZE
> - 1)
>
> /* RX FIFO constants */
> +#define MVPP2_RX_FIFO_PORT_DATA_SIZE_44KB0xb000
> #define MVPP2_RX_FIFO_PORT_DATA_SIZE_32KB0x8000
> #define MVPP2_RX_FIFO_PORT_DATA_SIZE_8KB 0x2000
> #define MVPP2_RX_FIFO_PORT_DATA_SIZE_4KB 0x1000
> -#define MVPP2_RX_FIFO_PORT_ATTR_SIZE_32KB0x200
> -#define MVPP2_RX_FIFO_PORT_ATTR_SIZE_8KB 0x80
> +#define MVPP2_RX_FIFO_PORT_ATTR_SIZE(data_size) ((data_size) >> 6)
> #define MVPP2_RX_FIFO_PORT_ATTR_SIZE_4KB 0x40
> #define MVPP2_RX_FIFO_PORT_MIN_PKT 0x80
>
> /* TX FIFO constants */
> -#define MVPP22_TX_FIFO_DATA_SIZE_10KB0xa
> -#define MVPP22_TX_FIFO_DATA_SIZE_3KB 0x3
> -#define MVPP2_TX_FIFO_THRESHOLD_MIN 256
> -#define MVPP2_TX_FIFO_THRESHOLD_10KB \
> - (MVPP22_TX_FIFO_DATA_SIZE_10KB * 1024 -
> MVPP2_TX_FIFO_THRESHOLD_MIN)
> -#define MVPP2_TX_FIFO_THRESHOLD_3KB \
> - (MVPP22_TX_FIFO_DATA_SIZE_3KB * 1024 -
> MVPP2_TX_FIFO_THRESHOLD_MIN)
> +#define MVPP22_TX_FIFO_DATA_SIZE_16KB16
> +#define MVPP22_TX_FIFO_DATA_SIZE_10KB10
> +#define MVPP22_TX_FIFO_DATA_SIZE_3KB 3
> +#define MVPP2_TX_FIFO_THRESHOLD_MIN 256 /* Bytes */
> +#define MVPP2_TX_FIFO_THRESHOLD(kb) \
> + ((kb) * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
>
> /* RX buffer constants */
> #define MVPP2_SKB_SHINFO_SIZE \
> @@ -946,6 +948,9 @@ struct mvpp2 {
> /* List of pointers to port structures */
> int port_count;
> struct mvpp2_port *port_list[MVPP2_MAX_PORTS];
> + /* Map of enabled ports */
> + unsigned long port_map;
> +
> struct mvpp2_tai *tai;
>
> /* Number of Tx threads used */
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index f6616c8..9ff5f57 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -6601,32 +6601,56 @@ static void mvpp2_rx_fifo_init(struct mvpp2 *priv)
> mvpp2_write(priv, MVPP2_RX_FIFO_INIT_REG, 0x1); }
>
> -static void mvpp22_rx_fifo_init(struct mvpp2 *priv)
> +static void mvpp22_rx_fifo_set_hw(struct mvpp2 *priv, int port, int
> +data_size)
> {
> - int port;
> + int attr_size = MVPP2_RX_FIFO_PORT_ATTR_SIZE(data_size);
>
> - /* The FIFO size parameters are set depending on the maximum speed
> a
> - * given port can handle:
> - * - Port 0: 10Gbps
> - * - Port 1: 2.5Gbps
> - * - Ports 2 and 3: 1Gbps
> - */
> + mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(port), data_size);
> + mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(port), attr_size); }
>
> - mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(0),
> - MVPP2_RX_FIFO_PORT_DATA_SIZE_32KB);
> - mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(0),
> - MVPP2_RX_FIFO_PORT_ATTR_SIZE_32KB);
> +/* Initialize TX FIFO's: the total FIFO size is 48kB on PPv2.2.
> + * 4kB fixed space must be assigned for the loopback port.
> + * Redistribute remaining avialable 44kB spac
Re: [PATCH RFC 04/12] vdpa: add vdpa simulator for block device
On Tue, Nov 17, 2020 at 06:38:11PM +0100, Stefano Garzarella wrote:
> On Tue, Nov 17, 2020 at 04:43:42PM +0000, Stefan Hajnoczi wrote:
> > On Tue, Nov 17, 2020 at 03:16:20PM +0100, Stefano Garzarella wrote:
> > > On Tue, Nov 17, 2020 at 11:11:21AM +, Stefan Hajnoczi wrote:
> > > > On Fri, Nov 13, 2020 at 02:47:04PM +0100, Stefano Garzarella wrote:
> > > > > +static void vdpasim_blk_work(struct work_struct *work)
> > > > > +{
> > > > > + struct vdpasim *vdpasim = container_of(work, struct vdpasim,
> > > > > work);
> > > > > + u8 status = VIRTIO_BLK_S_OK;
> > > > > + int i;
> > > > > +
> > > > > + spin_lock(>lock);
> > > > > +
> > > > > + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
> > > > > + goto out;
> > > > > +
> > > > > + for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) {
> > > > > + struct vdpasim_virtqueue *vq = >vqs[i];
> > > > > +
> > > > > + if (!vq->ready)
> > > > > + continue;
> > > > > +
> > > > > + while (vringh_getdesc_iotlb(>vring, >iov,
> > > > > >iov,
> > > > > + >head, GFP_ATOMIC) > 0)
> > > > > {
> > > > > +
> > > > > + int write;
> > > > > +
> > > > > + vq->iov.i = vq->iov.used - 1;
> > > > > + write = vringh_iov_push_iotlb(>vring,
> > > > > >iov, , 1);
> > > > > + if (write <= 0)
> > > > > + break;
> > > >
> > > > We're lucky the guest driver doesn't crash after VIRTIO_BLK_T_GET_ID? :)
> > >
> > > The crash could happen if the simulator doesn't put the string terminator,
> > > but in virtio_blk.c, the serial_show() initialize the buffer putting the
> > > string terminator in the VIRTIO_BLK_ID_BYTES element:
> > >
> > > buf[VIRTIO_BLK_ID_BYTES] = '\0';
> > > err = virtblk_get_id(disk, buf);
> > >
> > > This should prevent the issue, right?
> > >
> > > However in the last patch of this series I implemented VIRTIO_BLK_T_GET_ID
> > > support :-)
> >
> > Windows, BSD, macOS, etc guest drivers aren't necessarily going to
> > terminate or initialize the serial string buffer.
>
> Unfortunately I discovered that VIRTIO_BLK_T_GET_ID is not in the VIRTIO
> specs, so, just for curiosity, I checked the QEMU code and I found this:
>
> case VIRTIO_BLK_T_GET_ID:
> {
> /*
> * NB: per existing s/n string convention the string is
> * terminated by '\0' only when shorter than buffer.
> */
> const char *serial = s->conf.serial ? s->conf.serial : "";
> size_t size = MIN(strlen(serial) + 1,
> MIN(iov_size(in_iov, in_num),
> VIRTIO_BLK_ID_BYTES));
> iov_from_buf(in_iov, in_num, 0, serial, size);
> virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
> virtio_blk_free_request(req);
> break;
> }
>
> It seems that the device emulation in QEMU expects that the driver
> terminates the serial string buffer.
>
> Do you know why VIRTIO_BLK_T_GET_ID is not in the specs?
> Should we add it?
It's about to be merged into the VIRTIO spec:
https://github.com/oasis-tcs/virtio-spec/issues/63
Stefan
signature.asc
Description: PGP signature
Re: [PATCH RFC 04/12] vdpa: add vdpa simulator for block device
On Tue, Nov 17, 2020 at 03:16:20PM +0100, Stefano Garzarella wrote:
> On Tue, Nov 17, 2020 at 11:11:21AM +0000, Stefan Hajnoczi wrote:
> > On Fri, Nov 13, 2020 at 02:47:04PM +0100, Stefano Garzarella wrote:
> > > +static void vdpasim_blk_work(struct work_struct *work)
> > > +{
> > > + struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
> > > + u8 status = VIRTIO_BLK_S_OK;
> > > + int i;
> > > +
> > > + spin_lock(>lock);
> > > +
> > > + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
> > > + goto out;
> > > +
> > > + for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) {
> > > + struct vdpasim_virtqueue *vq = >vqs[i];
> > > +
> > > + if (!vq->ready)
> > > + continue;
> > > +
> > > + while (vringh_getdesc_iotlb(>vring, >iov, >iov,
> > > + >head, GFP_ATOMIC) > 0) {
> > > +
> > > + int write;
> > > +
> > > + vq->iov.i = vq->iov.used - 1;
> > > + write = vringh_iov_push_iotlb(>vring, >iov,
> > > , 1);
> > > + if (write <= 0)
> > > + break;
> >
> > We're lucky the guest driver doesn't crash after VIRTIO_BLK_T_GET_ID? :)
>
> The crash could happen if the simulator doesn't put the string terminator,
> but in virtio_blk.c, the serial_show() initialize the buffer putting the
> string terminator in the VIRTIO_BLK_ID_BYTES element:
>
> buf[VIRTIO_BLK_ID_BYTES] = '\0';
> err = virtblk_get_id(disk, buf);
>
> This should prevent the issue, right?
>
> However in the last patch of this series I implemented VIRTIO_BLK_T_GET_ID
> support :-)
Windows, BSD, macOS, etc guest drivers aren't necessarily going to
terminate or initialize the serial string buffer.
Anyway, the later patch that implements VIRTIO_BLK_T_GET_ID solves this
issue! Thanks.
Stefan
signature.asc
Description: PGP signature
Re: [PATCH RFC 12/12] vdpa_sim_blk: implement ramdisk behaviour
On Fri, Nov 13, 2020 at 02:47:12PM +0100, Stefano Garzarella wrote:
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index 8e41b3ab98d5..68e74383322f 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
>
> #include "vdpa_sim.h"
> @@ -24,10 +25,137 @@
>
> static struct vdpasim *vdpasim_blk_dev;
>
> +static int vdpasim_blk_handle_req(struct vdpasim *vdpasim,
> + struct vdpasim_virtqueue *vq)
This function has a non-standard int return value. Please document it.
> +{
> + size_t wrote = 0, to_read = 0, to_write = 0;
> + struct virtio_blk_outhdr hdr;
> + uint8_t status;
> + uint32_t type;
> + ssize_t bytes;
> + loff_t offset;
> + int i, ret;
> +
> + vringh_kiov_cleanup(>riov);
> + vringh_kiov_cleanup(>wiov);
> +
> + ret = vringh_getdesc_iotlb(>vring, >riov, >wiov,
> +>head, GFP_ATOMIC);
> + if (ret != 1)
> + return ret;
> +
> + for (i = 0; i < vq->wiov.used; i++)
> + to_write += vq->wiov.iov[i].iov_len;
> + to_write -= 1; /* last byte is the status */
What if vq->wiov.used == 0?
> +
> + for (i = 0; i < vq->riov.used; i++)
> + to_read += vq->riov.iov[i].iov_len;
> +
> + bytes = vringh_iov_pull_iotlb(>vring, >riov, , sizeof(hdr));
> + if (bytes != sizeof(hdr))
> + return 0;
> +
> + to_read -= bytes;
> +
> + type = le32_to_cpu(hdr.type);
> + offset = le64_to_cpu(hdr.sector) << SECTOR_SHIFT;
> + status = VIRTIO_BLK_S_OK;
> +
> + switch (type) {
> + case VIRTIO_BLK_T_IN:
> + if (offset + to_write > VDPASIM_BLK_CAPACITY << SECTOR_SHIFT) {
Integer overflow is not handled.
signature.asc
Description: PGP signature
Re: [PATCH RFC 10/12] vdpa_sim: split vdpasim_virtqueue's iov field in riov and wiov
On Fri, Nov 13, 2020 at 02:47:10PM +0100, Stefano Garzarella wrote: > vringh_getdesc_iotlb() manages 2 iovs for writable and readable > descriptors. This is very useful for the block device, where for > each request we have both types of descriptor. > > Let's split the vdpasim_virtqueue's iov field in riov and wiov > to use them with vringh_getdesc_iotlb(). Is riov/wiov naming common? VIRTIO uses "in" (device-to-driver) and "out" (driver-to-device). Using VIRTIO terminology might be clearer. Stefan signature.asc Description: PGP signature
Re: [PATCH RFC 06/12] vdpa_sim: add struct vdpasim_device to store device properties
On Fri, Nov 13, 2020 at 02:47:06PM +0100, Stefano Garzarella wrote:
> Move device properties used during the entire life cycle in a new
> structure to simplify the copy of these fields during the vdpasim
> initialization.
>
> Signed-off-by: Stefano Garzarella
> ---
> drivers/vdpa/vdpa_sim/vdpa_sim.h | 17 --
> drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 ++--
> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 8 +--
> drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 9 +---
> 4 files changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.h
> b/drivers/vdpa/vdpa_sim/vdpa_sim.h
> index 6a1267c40d5e..76e642042eb0 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim.h
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.h
> @@ -40,12 +40,17 @@ struct vdpasim_virtqueue {
> irqreturn_t (*cb)(void *data);
> };
>
> +struct vdpasim_device {
> + u64 supported_features;
> + u32 id;
> + int nvqs;
> +};
> +
> struct vdpasim_init_attr {
> - u32 device_id;
> - u64 features;
> + struct vdpasim_device device;
It's unclear to me what the exact purpose of struct vdpasim_device is.
At least the name reminds me of struct device, which this is not.
Should this be called just struct vdpasim_attr or struct
vdpasim_dev_attr? In other words, the attributes that are needed even
after intialization?
signature.asc
Description: PGP signature
Re: [PATCH RFC 04/12] vdpa: add vdpa simulator for block device
On Fri, Nov 13, 2020 at 02:47:04PM +0100, Stefano Garzarella wrote:
> +static void vdpasim_blk_work(struct work_struct *work)
> +{
> + struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
> + u8 status = VIRTIO_BLK_S_OK;
> + int i;
> +
> + spin_lock(>lock);
> +
> + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
> + goto out;
> +
> + for (i = 0; i < VDPASIM_BLK_VQ_NUM; i++) {
> + struct vdpasim_virtqueue *vq = >vqs[i];
> +
> + if (!vq->ready)
> + continue;
> +
> + while (vringh_getdesc_iotlb(>vring, >iov, >iov,
> + >head, GFP_ATOMIC) > 0) {
> +
> + int write;
> +
> + vq->iov.i = vq->iov.used - 1;
> + write = vringh_iov_push_iotlb(>vring, >iov,
> , 1);
> + if (write <= 0)
> + break;
We're lucky the guest driver doesn't crash after VIRTIO_BLK_T_GET_ID? :)
signature.asc
Description: PGP signature
Re: [PATCH RFC 01/12] vhost-vdpa: add support for vDPA blk devices
On Fri, Nov 13, 2020 at 02:47:01PM +0100, Stefano Garzarella wrote: > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index 2754f3069738..fb0411594963 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > > #include "vhost.h" > > @@ -194,6 +195,9 @@ static int vhost_vdpa_config_validate(struct vhost_vdpa > *v, > case VIRTIO_ID_NET: > size = sizeof(struct virtio_net_config); > break; > + case VIRTIO_ID_BLOCK: > + size = sizeof(struct virtio_blk_config); > + break; > } > > if (c->len == 0) Can vdpa_config_ops->get/set_config() handle the size check instead of hardcoding device-specific knowledge into drivers/vhost/vdpa.c? signature.asc Description: PGP signature
Re: [PATCH] mm/zsmalloc: include sparsemem.h for MAX_PHYSMEM_BITS
[adding Russell King for ARM]
On 2020-11-10 12:21, Arnd Bergmann wrote:
> On Tue, Nov 10, 2020 at 10:58 AM Mike Rapoport wrote:
>> > >
>> > > asm/sparsemem.h is not available on some architectures.
>> > > It's better to use linux/mmzone.h instead.
>
> Ah, I missed that, too.
>
>> > Hm, linux/mmzone.h only includes asm/sparsemem.h when CONFIG_SPARSEMEM
>> > is enabled. However, on ARM at least I can have configurations without
>> > CONFIG_SPARSEMEM and physical address extension on (e.g.
>> > multi_v7_defconfig + CONFIG_LPAE + CONFIG_ZSMALLOC).
>> >
>> > While sparsemem seems to be a good idea with LPAE it really seems not
>> > required (see also https://lore.kernel.org/patchwork/patch/567589/).
>> >
>> > There seem to be also other architectures which define MAX_PHYSMEM_BITS
>> > only when SPARSEMEM is enabled, e.g.
>> > arch/riscv/include/asm/sparsemem.h...
>> >
>> > Not sure how to get out of this.. Maybe make ZSMALLOC dependent on
>> > SPARSEMEM? It feels a bit silly restricting ZSMALLOC selection only due
>> > to a compile time define...
>>
>> I think we can define MAX_POSSIBLE_PHYSMEM_BITS in one of
>> arch/arm/inclide/asm/pgtable-{2,3}level-*.h headers to values supported
>> by !LPAE and LPAE.
Hm I see mm/zsmalloc.c really only needs to know how many bits are
potentially used to calculate how many bits it can use for object
indexing.
>
> Good idea. I wonder what other architectures need the same though.
> Here are some I found:
>
> $ git grep -l PHYS_ADDR_T_64BIT arch | grep Kconfig
> arch/arc/Kconfig
> arch/arm/mm/Kconfig
> arch/mips/Kconfig
> arch/powerpc/platforms/Kconfig.cputype
> arch/x86/Kconfig
>
> arch/arc has a CONFIG_ARC_HAS_PAE40 option
> arch/riscv has 34-bit addressing in rv32 mode
> arch/mips has up to 40 bits with mips32r3 XPA, but I don't know what
> supports that
>
> arch/powerpc has this:
> config PHYS_64BIT
> bool 'Large physical address support' if E500 || PPC_86xx
> depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx
>
> Apparently all three (4xx, e500v2, mpc86xx/e600) do 36-bit physical
> addressing, but each one has a different page table format.
>
> Microblaze has physical address extensions, but neither those nor
> 64-bit mode have so far made it into the kernel.
>
> To be on the safe side, we could provoke a compile-time error
> when CONFIG_PHYS_ADDR_T_64BIT is set on a 32-bit
> architecture, but MAX_POSSIBLE_PHYSMEM_BITS is not set.
>
>> That's what x86 does:
>>
>> $ git grep -w MAX_POSSIBLE_PHYSMEM_BITS arch/
>> arch/x86/include/asm/pgtable-3level_types.h:#define
>> MAX_POSSIBLE_PHYSMEM_BITS 36
>
> Doesn't x86 also support a 40-bit addressing mode? I suppose
> those machines that actually used it are long gone.
>
>> arch/x86/include/asm/pgtable_64_types.h:#define MAX_POSSIBLE_PHYSMEM_BITS
>>52
>>
>> It seems that actual numbers would be 36 for !LPAE and 40 for LPAE, but
>> I'm not sure about that.
>
> Close enough, yes.
>
> The 36-bit addressing is on !LPAE is only used for early static mappings,
> so I think we can pretend it's always 32-bit. I checked the ARMv8 reference,
> and it says that ARMv8-Aarch32 actually supports 40 bit physical addressing
> both with non-LPAE superpages (short descriptor format) and LPAE (long
> descriptor format), but Linux only does 36-bit addressing on superpages
> as specified for ARMv6/ARMv7 short descriptors.
Oh so, more than 4GB of memory can be supported by !LPAE systems via
superpages? Wasn't aware of that.
Since only ARM_LPAE selects CONFIG_PHYS_ADDR_T_64BIT it really is safe
to assume 32 bits for non-LPAE systems.
I guess that would mean adding a #define MAX_POSSIBLE_PHYSMEM_BITS 32 to
arch/arm/include/asm/pgtable-2level.h and a MAX_POSSIBLE_PHYSMEM_BITS 40
in arch/arm/include/asm/pgtable-3level.h. Seems straight forward and
would solve the problem I had. I can prepare a patch for ARM, not sure
about the other architectures...
--
Stefan
Re: [PATCH] mm/zsmalloc: include sparsemem.h for MAX_PHYSMEM_BITS
On 2020-11-08 07:46, Mike Rapoport wrote:
> On Sat, Nov 07, 2020 at 04:22:06PM +0100, Stefan Agner wrote:
>> Most architectures define MAX_PHYSMEM_BITS in asm/sparsemem.h and don't
>> include it in asm/pgtable.h. Include asm/sparsemem.h directly to get
>> the MAX_PHYSMEM_BITS define on all architectures.
>>
>> This fixes a crash when accessing zram on 32-bit ARM platform with LPAE and
>> more than 4GB of memory:
>> Unable to handle kernel NULL pointer dereference at virtual address
>>
>> pgd = a27bd01c
>> [] *pgd=236a0003, *pmd=1ffa64003
>> Internal error: Oops: 207 [#1] SMP ARM
>> Modules linked in: mdio_bcm_unimac(+) brcmfmac cfg80211 brcmutil
>> raspberrypi_hwmon hci_uart crc32_arm_ce bcm2711_thermal phy_generic genet
>> CPU: 0 PID: 123 Comm: mkfs.ext4 Not tainted 5.9.6 #1
>> Hardware name: BCM2711
>> PC is at zs_map_object+0x94/0x338
>> LR is at zram_bvec_rw.constprop.0+0x330/0xa64
>> pc : []lr : []psr: 6013
>> sp : e376bbe0 ip : fp : c1e2921c
>> r10: 0002 r9 : c1dda730 r8 :
>> r7 : e8ff7a00 r6 : r5 : 02f9ffa0 r4 : e371
>> r3 : 000fdffe r2 : c1e0ce80 r1 : ebf979a0 r0 :
>> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>> Control: 30c5383d Table: 235c2a80 DAC: fffd
>> Process mkfs.ext4 (pid: 123, stack limit = 0x495a22e6)
>> Stack: (0xe376bbe0 to 0xe376c000)
>> ...
>> [] (zs_map_object) from []
>> (zram_bvec_rw.constprop.0+0x330/0xa64)
>> [] (zram_bvec_rw.constprop.0) from []
>> (zram_submit_bio+0x1a4/0x40c)
>> [] (zram_submit_bio) from []
>> (submit_bio_noacct+0xd0/0x3c8)
>> [] (submit_bio_noacct) from [] (submit_bio+0x4c/0x190)
>> [] (submit_bio) from [] (submit_bh_wbc+0x188/0x1b8)
>> [] (submit_bh_wbc) from []
>> (__block_write_full_page+0x340/0x5e4)
>> [] (__block_write_full_page) from []
>> (block_write_full_page+0x128/0x170)
>> [] (block_write_full_page) from []
>> (__writepage+0x14/0x68)
>> [] (__writepage) from []
>> (write_cache_pages+0x1bc/0x494)
>> [] (write_cache_pages) from []
>> (generic_writepages+0x58/0x8c)
>> [] (generic_writepages) from []
>> (do_writepages+0x48/0xec)
>> [] (do_writepages) from []
>> (__filemap_fdatawrite_range+0xf0/0x128)
>> [] (__filemap_fdatawrite_range) from []
>> (file_write_and_wait_range+0x48/0x98)
>> [] (file_write_and_wait_range) from []
>> (blkdev_fsync+0x1c/0x44)
>> [] (blkdev_fsync) from [] (do_fsync+0x3c/0x70)
>> [] (do_fsync) from [] (__sys_trace_return+0x0/0x2c)
>> Exception stack(0xe376bfa8 to 0xe376bff0)
>> bfa0: 0003d2e0 b6f7b6f0 0003 00046e40 1000
>>
>> bfc0: 0003d2e0 b6f7b6f0 0076 befcbb20
>> befcbb28
>> bfe0: b6f4e060 befcbad8 b6f23e0c b6dc4a80
>> Code: e5927000 e0050391 e0871005 e5918018 (e5983000)
>>
>> Fixes: 61989a80fb3a ("staging: zsmalloc: zsmalloc memory allocation library")
>> Signed-off-by: Stefan Agner
>> ---
>> mm/zsmalloc.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> index c36fdff9a371..260bd48aacd0 100644
>> --- a/mm/zsmalloc.c
>> +++ b/mm/zsmalloc.c
>> @@ -40,6 +40,7 @@
>> #include
>> #include
>> #include
>> +#include
>
> asm/sparsemem.h is not available on some architectures.
> It's better to use linux/mmzone.h instead.
>
Hm, linux/mmzone.h only includes asm/sparsemem.h when CONFIG_SPARSEMEM
is enabled. However, on ARM at least I can have configurations without
CONFIG_SPARSEMEM and physical address extension on (e.g.
multi_v7_defconfig + CONFIG_LPAE + CONFIG_ZSMALLOC).
While sparsemem seems to be a good idea with LPAE it really seems not
required (see also https://lore.kernel.org/patchwork/patch/567589/).
There seem to be also other architectures which define MAX_PHYSMEM_BITS
only when SPARSEMEM is enabled, e.g.
arch/riscv/include/asm/sparsemem.h...
Not sure how to get out of this.. Maybe make ZSMALLOC dependent on
SPARSEMEM? It feels a bit silly restricting ZSMALLOC selection only due
to a compile time define...
--
Stefan
>> #include
>> #include
>> #include
>> --
>> 2.29.1
>>
>>
Re: [PATCH] mm/zsmalloc: include sparsemem.h for MAX_PHYSMEM_BITS
On 2020-11-08 01:56, Andrew Morton wrote: > On Sat, 7 Nov 2020 16:22:06 +0100 Stefan Agner wrote: > >> Most architectures define MAX_PHYSMEM_BITS in asm/sparsemem.h and don't >> include it in asm/pgtable.h. Include asm/sparsemem.h directly to get >> the MAX_PHYSMEM_BITS define on all architectures. >> >> This fixes a crash when accessing zram on 32-bit ARM platform with LPAE and >> more than 4GB of memory: >> Unable to handle kernel NULL pointer dereference at virtual address >> > > Mysterious. Presumably without this include, some compilation unit is > picking up the wrong value of MAX_PHYSMEM_BITS? But I couldn't > actually see where/how this occurs. Can you please explain further? Not sure if I got that right, but from what I understand if MAX_PHYSMEM_BITS is not set in mm/zsmalloc.c it will set MAX_PHYSMEM_BITS to BITS_PER_LONG. And this is 32-bit, too short when LPAE is in use... -- Stefan
[PATCH] mm/zsmalloc: include sparsemem.h for MAX_PHYSMEM_BITS
Most architectures define MAX_PHYSMEM_BITS in asm/sparsemem.h and don't
include it in asm/pgtable.h. Include asm/sparsemem.h directly to get
the MAX_PHYSMEM_BITS define on all architectures.
This fixes a crash when accessing zram on 32-bit ARM platform with LPAE and
more than 4GB of memory:
Unable to handle kernel NULL pointer dereference at virtual address
pgd = a27bd01c
[] *pgd=236a0003, *pmd=1ffa64003
Internal error: Oops: 207 [#1] SMP ARM
Modules linked in: mdio_bcm_unimac(+) brcmfmac cfg80211 brcmutil
raspberrypi_hwmon hci_uart crc32_arm_ce bcm2711_thermal phy_generic genet
CPU: 0 PID: 123 Comm: mkfs.ext4 Not tainted 5.9.6 #1
Hardware name: BCM2711
PC is at zs_map_object+0x94/0x338
LR is at zram_bvec_rw.constprop.0+0x330/0xa64
pc : []lr : []psr: 6013
sp : e376bbe0 ip : fp : c1e2921c
r10: 0002 r9 : c1dda730 r8 :
r7 : e8ff7a00 r6 : r5 : 02f9ffa0 r4 : e371
r3 : 000fdffe r2 : c1e0ce80 r1 : ebf979a0 r0 :
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 30c5383d Table: 235c2a80 DAC: fffd
Process mkfs.ext4 (pid: 123, stack limit = 0x495a22e6)
Stack: (0xe376bbe0 to 0xe376c000)
...
[] (zs_map_object) from []
(zram_bvec_rw.constprop.0+0x330/0xa64)
[] (zram_bvec_rw.constprop.0) from []
(zram_submit_bio+0x1a4/0x40c)
[] (zram_submit_bio) from []
(submit_bio_noacct+0xd0/0x3c8)
[] (submit_bio_noacct) from [] (submit_bio+0x4c/0x190)
[] (submit_bio) from [] (submit_bh_wbc+0x188/0x1b8)
[] (submit_bh_wbc) from []
(__block_write_full_page+0x340/0x5e4)
[] (__block_write_full_page) from []
(block_write_full_page+0x128/0x170)
[] (block_write_full_page) from [] (__writepage+0x14/0x68)
[] (__writepage) from [] (write_cache_pages+0x1bc/0x494)
[] (write_cache_pages) from []
(generic_writepages+0x58/0x8c)
[] (generic_writepages) from [] (do_writepages+0x48/0xec)
[] (do_writepages) from []
(__filemap_fdatawrite_range+0xf0/0x128)
[] (__filemap_fdatawrite_range) from []
(file_write_and_wait_range+0x48/0x98)
[] (file_write_and_wait_range) from []
(blkdev_fsync+0x1c/0x44)
[] (blkdev_fsync) from [] (do_fsync+0x3c/0x70)
[] (do_fsync) from [] (__sys_trace_return+0x0/0x2c)
Exception stack(0xe376bfa8 to 0xe376bff0)
bfa0: 0003d2e0 b6f7b6f0 0003 00046e40 1000
bfc0: 0003d2e0 b6f7b6f0 0076 befcbb20 befcbb28
bfe0: b6f4e060 befcbad8 b6f23e0c b6dc4a80
Code: e5927000 e0050391 e0871005 e5918018 (e5983000)
Fixes: 61989a80fb3a ("staging: zsmalloc: zsmalloc memory allocation library")
Signed-off-by: Stefan Agner
---
mm/zsmalloc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index c36fdff9a371..260bd48aacd0 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include
#include
#include
--
2.29.1
Re: [PATCH] net/ieee802154: remove unused macros to tame gcc
Hello. On 06.11.20 09:10, Alex Shi wrote: Signed-off-by: Alex Shi Cc: Alexander Aring Cc: Stefan Schmidt Cc: "David S. Miller" Cc: Jakub Kicinski Cc: linux-w...@vger.kernel.org Cc: net...@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- net/ieee802154/nl802154.c | 4 1 file changed, 4 deletions(-) diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c index 7c5a1aa5adb4..1cebdcedc48c 100644 --- a/net/ieee802154/nl802154.c +++ b/net/ieee802154/nl802154.c @@ -2098,11 +2098,7 @@ static int nl802154_del_llsec_seclevel(struct sk_buff *skb, #define NL802154_FLAG_NEED_NETDEV 0x02 #define NL802154_FLAG_NEED_RTNL 0x04 #define NL802154_FLAG_CHECK_NETDEV_UP 0x08 -#define NL802154_FLAG_NEED_NETDEV_UP (NL802154_FLAG_NEED_NETDEV |\ -NL802154_FLAG_CHECK_NETDEV_UP) #define NL802154_FLAG_NEED_WPAN_DEV 0x10 -#define NL802154_FLAG_NEED_WPAN_DEV_UP (NL802154_FLAG_NEED_WPAN_DEV |\ -NL802154_FLAG_CHECK_NETDEV_UP) static int nl802154_pre_doit(const struct genl_ops *ops, struct sk_buff *skb, struct genl_info *info) This patch has been applied to the wpan tree and will be part of the next pull request to net. Thanks! regards Stefan Schmidt
Re: [PATCH] vhost/vsock: add IOTLB API support
On Thu, Oct 29, 2020 at 06:43:51PM +0100, Stefano Garzarella wrote: > This patch enables the IOTLB API support for vhost-vsock devices, > allowing the userspace to emulate an IOMMU for the guest. > > These changes were made following vhost-net, in details this patch: > - exposes VIRTIO_F_ACCESS_PLATFORM feature and inits the iotlb > device if the feature is acked > - implements VHOST_GET_BACKEND_FEATURES and > VHOST_SET_BACKEND_FEATURES ioctls > - calls vq_meta_prefetch() before vq processing to prefetch vq > metadata address in IOTLB > - provides .read_iter, .write_iter, and .poll callbacks for the > chardev; they are used by the userspace to exchange IOTLB messages > > This patch was tested with QEMU and a patch applied [1] to fix a > simple issue: > $ qemu -M q35,accel=kvm,kernel-irqchip=split \ >-drive file=fedora.qcow2,format=qcow2,if=virtio \ >-device intel-iommu,intremap=on \ >-device vhost-vsock-pci,guest-cid=3,iommu_platform=on > > [1] https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg09077.html > > Signed-off-by: Stefano Garzarella > --- > drivers/vhost/vsock.c | 68 +++++-- > 1 file changed, 65 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Hello
Greetings my beloved, My name is Mrs.Julianna Stefan Ndoi,I am a deaf woman and also a cancer patient who had decided to donate what I have to you for God's works. I want to donate $8.5 million to you so that you will use part of the this fund to help the poor ones,while you use the rest for your family.If you are interested,Respond now for more details on how to receive this fund. Regards, Mrs.Julianna,greeting from the sick bed ** Saludos mi amado, Mi nombre es Sra. Julianna Stefan Ndoi, soy una hermana sorda ... Soy una paciente de c?ncer que ten?a Decid? donarles lo que tengo para las obras de Dios. Quiero donar $ 8.5 millones para usted para que use parte de este fondo para ayudar los pobres, mientras que el resto lo usas para tu familia. interesado, responda ahora para obtener m?s detalles sobre c?mo recibir este fondo. Saludos, Se?ora Julianna, saludando desde la cama de enferma
Re: [PATCH] Bluetooth: A2MP: Do not set rsp.id to zero
Am Sonntag, den 18.10.2020, 10:05 +0200 schrieb Stefan Gottwald:
> Due to security reasons the rsp struct is not zerod out in one case this will
> also zero out the former set rsp.id which seems to be wrong.
>
> Signed-off-by: Stefan Gottwald
> ---
> net/bluetooth/a2mp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> index da7fd7c..7a1e0b7 100644
> --- a/net/bluetooth/a2mp.c
> +++ b/net/bluetooth/a2mp.c
> @@ -381,10 +381,11 @@ static int a2mp_getampassoc_req(struct amp_mgr *mgr,
> struct sk_buff *skb,
> hdev = hci_dev_get(req->id);
> if (!hdev || hdev->amp_type == AMP_TYPE_BREDR || tmp) {
> struct a2mp_amp_assoc_rsp rsp;
> - rsp.id = req->id;
>
> memset(, 0, sizeof(rsp));
>
> + rsp.id = req->id;
> +
> if (tmp) {
> rsp.status = A2MP_STATUS_COLLISION_OCCURED;
> amp_mgr_put(tmp);
As it seems I'm too slow there is already a fix from the author of the initial
patch.
https://lore.kernel.org/linux-bluetooth/20201016180956.707681-2-luiz.de...@gmail.com/
There is a additional patch in this series which might also be a important fix
https://lore.kernel.org/linux-bluetooth/20201016180956.707681-1-luiz.de...@gmail.com/
Thanks to a LWN member pointing this out to me.
[PATCH] Bluetooth: A2MP: Do not set rsp.id to zero
Due to security reasons the rsp struct is not zerod out in one case this will
also zero out the former set rsp.id which seems to be wrong.
Signed-off-by: Stefan Gottwald
---
net/bluetooth/a2mp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
index da7fd7c..7a1e0b7 100644
--- a/net/bluetooth/a2mp.c
+++ b/net/bluetooth/a2mp.c
@@ -381,10 +381,11 @@ static int a2mp_getampassoc_req(struct amp_mgr *mgr,
struct sk_buff *skb,
hdev = hci_dev_get(req->id);
if (!hdev || hdev->amp_type == AMP_TYPE_BREDR || tmp) {
struct a2mp_amp_assoc_rsp rsp;
- rsp.id = req->id;
memset(, 0, sizeof(rsp));
+ rsp.id = req->id;
+
if (tmp) {
rsp.status = A2MP_STATUS_COLLISION_OCCURED;
amp_mgr_put(tmp);
--
2.7.4
Re: [Intel-gfx] [PATCH] Revert "drm/i915: Force state->modeset=true when distrust_bios_wm==true"
Dear Ville Syrjälä, Thank you for responding so quickly. I was occupied with work and life for the past two weeks, sorry about the wait, but have now managed to find some time to continue pursuing this issue again. On Thu, 2020-10-01 at 18:23 +0300, Ville Syrjälä wrote: > Argh. If only I had managed to land the full dbuf rework and nuke > this mess before it came back to bite us... Yeah... I know the feeling. > This is definitely going to break something else, so not great. I did expect that. The automated tests failing was a pretty good indicator. You put that code in to take care of something. All I did was just tear it down because it happens to work better for me... blunt but functional for my purposes. Seems to need some finesse or a condition to not cause my problem? Yet still function for the reason you put it in there for. I am more than willing to play guinea pig and test patches on my end to improve it with you. > Can you file an upstream bug at > https://gitlab.freedesktop.org/drm/intel/issues/new > and attach dmesgs from booting both good and bad kernels with > drm.debug=0x1e passed to the kernel cmdline? Bump log_buf_len= > if necessary to capture the full log. I have done so today. It's at: https://gitlab.freedesktop.org/drm/intel/-/issues/2579 -- Stefan Joosten AT Computing BV Sharing and growing together Tel +31 24 352 72 82 Arnhemsestraatweg 33 i...@atcomputing.nl 6881 ND Velp www.atcomputing.nl signature.asc Description: This is a digitally signed message part
Re: [PATCH v2 5/5] media: mt9p031: Fix corrupted frame after restarting stream
Hi Laurent,
On 05.10.20 15:08, Laurent Pinchart wrote:
Hi Stefan,
On Mon, Oct 05, 2020 at 11:28:21AM +0200, Stefan Riedmüller wrote:
On 02.10.20 02:05, Laurent Pinchart wrote:
On Wed, Sep 30, 2020 at 12:51:33PM +0200, Stefan Riedmueller wrote:
From: Dirk Bender
To prevent corrupted frames after starting and stopping the sensor it's
s/it's/its/
thanks, I'll fix that.
datasheet specifies a specific pause sequence to follow:
Stopping:
Set Pause_Restart Bit -> Set Restart Bit -> Set Chip_Enable Off
Restarting:
Set Chip_Enable On -> Clear Pause_Restart Bit
The Restart Bit is cleared automatically and must not be cleared
manually as this would cause undefined behavior.
Signed-off-by: Dirk Bender
Signed-off-by: Stefan Riedmueller
---
No changes in v2
---
drivers/media/i2c/mt9p031.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index d10457361e6c..d59f66e3dcf3 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -80,6 +80,8 @@
#define MT9P031_PIXEL_CLOCK_SHIFT(n)((n) << 8)
#define MT9P031_PIXEL_CLOCK_DIVIDE(n) ((n) << 0)
#define MT9P031_FRAME_RESTART0x0b
+#defineMT9P031_FRAME_RESTART_SET (1 << 0)
+#defineMT9P031_FRAME_PAUSE_RESTART_SET (1 << 1)
The fields are named Restart and Pause_Restart, I would drop _SET. Could
you also sort them from MSB to LSB as for the other registers ? Using
BIT() would be good too, although this could be done as an additional
patch to convert all the existing macros.
I'll do that. Also I will rename the register to MT9P031_RESTART and the
bits to MT9P031_FRAME_RESTART and MT9P031_FRAME_PAUSE_RESTART.
#define MT9P031_SHUTTER_DELAY0x0c
#define MT9P031_RST 0x0d
#define MT9P031_RST_ENABLE 1
@@ -483,9 +485,25 @@ static int mt9p031_set_params(struct mt9p031 *mt9p031)
static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable)
{
struct mt9p031 *mt9p031 = to_mt9p031(subdev);
+ struct i2c_client *client = v4l2_get_subdevdata(subdev);
+ int val;
int ret;
if (!enable) {
+ val = mt9p031_read(client, MT9P031_FRAME_RESTART);
Do you need to read the register ? Can't you write
MT9P031_FRAME_PAUSE_RESTART_SET and then MT9P031_FRAME_PAUSE_RESTART_SET
| MT9P031_FRAME_RESTART_SET ? And actually, can't we just write both
bits in one go, do we need two writes ?
I think you're right we don't necessarily need to read the registers. The
only other bit is not used by the driver.
But I think we do need two separate writes, at least that is what the
datasheet states.
So I would drop the read but keep both write, ok?
That's fine with me if required, although I don't see where this is
indicated in the datasheet, but I may have missed it.
It's in "Standby and Chip Enable". There is a Sequence for entering soft
standby with two separate writes:
REG = 0x0B, 0x0002
REG = 0x0B, 0x0003
Regards,
Stefan
+
+ /* enable pause restart */
+ val |= MT9P031_FRAME_PAUSE_RESTART_SET;
+ ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val);
+ if (ret < 0)
+ return ret;
+
+ /* enable restart + keep pause restart set */
+ val |= MT9P031_FRAME_RESTART_SET;
+ ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val);
+ if (ret < 0)
+ return ret;
+
/* Stop sensor readout */
ret = mt9p031_set_output_control(mt9p031,
MT9P031_OUTPUT_CONTROL_CEN, 0);
@@ -505,6 +523,13 @@ static int mt9p031_s_stream(struct v4l2_subdev *subdev,
int enable)
if (ret < 0)
return ret;
+ val = mt9p031_read(client, MT9P031_FRAME_RESTART);
+ /* disable reset + pause restart */
+ val &= ~MT9P031_FRAME_PAUSE_RESTART_SET;
Same here, I think you can simply write MT9P031_FRAME_PAUSE_RESTART_SET.
I'll drop the read here as well. But I need to make sure, that the Restart
Bit is not cleared manually here.
+ ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val);
+ if (ret < 0)
+ return ret;
+
return mt9p031_pll_enable(mt9p031);
}
Re: [PATCH v4 2/2] s390/dasd: remove ioctl_by_bdev calls
Am 07.10.20 um 12:44 schrieb Christian Borntraeger: > > On 07.10.20 12:39, Christoph Hellwig wrote: >> On Wed, Oct 07, 2020 at 11:34:17AM +0200, Christian Borntraeger wrote: >>> On 19.05.20 16:22, Stefan Haberland wrote: >>>> The IBM partition parser requires device type specific information only >>>> available to the DASD driver to correctly register partitions. The >>>> current approach of using ioctl_by_bdev with a fake user space pointer >>>> is discouraged. >>>> >>>> Fix this by replacing IOCTL calls with direct in-kernel function calls. >>>> >>>> Suggested-by: Christoph Hellwig >>>> Signed-off-by: Stefan Haberland >>>> Reviewed-by: Jan Hoeppner >>>> Reviewed-by: Peter Oberparleiter >>> FWIW, this broken the ibm-partition code for virtio-blk, when CONFIG_DASD=m. >> What are the symptoms? > During boot I normally have > > [0.930231] virtio_blk virtio1: [vda] 5409180 4096-byte logical blocks > (22.2 GB/20.6 GiB) > [0.930233] vda: detected capacity change from 0 to 22156001280 > [0.932806] vda:VOL1/ 0X: vda1 vda2 vda3 > > With this change, the last line is no longer there (if CONFIG_DASD=m) and > this also > reflects itself in /proc/partitions. The partitions are no longer detected. OK, looks like the module is not loaded and with fn = symbol_get(dasd_biodasdinfo); if (!fn) goto out_exit; the ibm.c partition detection aborts. Solution could be not to exit in this case but jump to the probing process. I will have a closer look at this.
Re: [PATCH v3 0/3] Support NVIDIA Tegra-based Ouya game console
Hi Peter, On 2020-10-04 15:31, Peter Geis wrote: > Good Day, > > This series introduces upstream kernel support for the Ouya game > console device. Please review and apply. Thank you in advance. Interesting patchset, maybe I can give my Ouya a second live now :-) Do you happen to have (a link) to instructions how to flash the device? Btw, there was also a driver for the Bluetooth controller on the ML once, maybe a good time to revive that: https://spinics.net/lists/linux-input/msg56288.html -- Stefan > > Changelog: > v3: - Reorder aliases per Dmitry Osipenko's review. > - Add sdio clocks per Dmitry Osipenko's review. > - Add missing ti sleep bits per Dmitry Osipenko's review. > - Enable lp1 sleep mode. > - Fix bluetooth comment and add missing power supplies. > > v2: - Update pmic and clock handles per Rob Herring's review. > - Add acks from Rob Herring to patch 2 and 3. > > Peter Geis (3): > ARM: tegra: Add device-tree for Ouya > dt-bindings: Add vendor prefix for Ouya Inc. > dt-bindings: ARM: tegra: Add Ouya game console > > .../devicetree/bindings/arm/tegra.yaml|3 + > .../devicetree/bindings/vendor-prefixes.yaml |2 + > arch/arm/boot/dts/Makefile|3 +- > arch/arm/boot/dts/tegra30-ouya.dts| 4511 + > 4 files changed, 4518 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/boot/dts/tegra30-ouya.dts
Re: [PATCH v5 25/52] docs: net: ieee802154.rst: fix C expressions
Hello.
[Sorry for the earlier empty mail]
On 06.10.20 16:03, Mauro Carvalho Chehab wrote:
There are some warnings produced with Sphinx 3.x:
Documentation/networking/ieee802154.rst:29: WARNING: Error in
declarator or parameters
Invalid C declaration: Expecting "(" in parameters. [error at 7]
int sd = socket(PF_IEEE802154, SOCK_DGRAM, 0);
---^
Documentation/networking/ieee802154.rst:134: WARNING: Invalid C
declaration: Expected end of definition. [error at 81]
void ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff
*skb, u8 lqi):
-^
Documentation/networking/ieee802154.rst:139: WARNING: Invalid C
declaration: Expected end of definition. [error at 95]
void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct
sk_buff *skb, bool ifs_handling):
---^
Documentation/networking/ieee802154.rst:158: WARNING: Invalid C
declaration: Expected end of definition. [error at 35]
int start(struct ieee802154_hw *hw):
---^
Documentation/networking/ieee802154.rst:162: WARNING: Invalid C
declaration: Expected end of definition. [error at 35]
void stop(struct ieee802154_hw *hw):
---^
Documentation/networking/ieee802154.rst:166: WARNING: Invalid C
declaration: Expected end of definition. [error at 61]
int xmit_async(struct ieee802154_hw *hw, struct sk_buff *skb):
-^
Documentation/networking/ieee802154.rst:171: WARNING: Invalid C
declaration: Expected end of definition. [error at 43]
int ed(struct ieee802154_hw *hw, u8 *level):
---^
Documentation/networking/ieee802154.rst:176: WARNING: Invalid C
declaration: Expected end of definition. [error at 62]
int set_channel(struct ieee802154_hw *hw, u8 page, u8 channel):
--^
Caused by some bad c:function: prototypes. Fix them.
Acked-by: David S. Miller
Signed-off-by: Mauro Carvalho Chehab
---
Documentation/networking/ieee802154.rst | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
Acked-by: Stefan Schmidt
regards
Stefan Schmidt
Re: [PATCH v5 25/52] docs: net: ieee802154.rst: fix C expressions
On 06.10.20 16:03, Mauro Carvalho Chehab wrote:
There are some warnings produced with Sphinx 3.x:
Documentation/networking/ieee802154.rst:29: WARNING: Error in
declarator or parameters
Invalid C declaration: Expecting "(" in parameters. [error at 7]
int sd = socket(PF_IEEE802154, SOCK_DGRAM, 0);
---^
Documentation/networking/ieee802154.rst:134: WARNING: Invalid C
declaration: Expected end of definition. [error at 81]
void ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff
*skb, u8 lqi):
-^
Documentation/networking/ieee802154.rst:139: WARNING: Invalid C
declaration: Expected end of definition. [error at 95]
void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct
sk_buff *skb, bool ifs_handling):
---^
Documentation/networking/ieee802154.rst:158: WARNING: Invalid C
declaration: Expected end of definition. [error at 35]
int start(struct ieee802154_hw *hw):
---^
Documentation/networking/ieee802154.rst:162: WARNING: Invalid C
declaration: Expected end of definition. [error at 35]
void stop(struct ieee802154_hw *hw):
---^
Documentation/networking/ieee802154.rst:166: WARNING: Invalid C
declaration: Expected end of definition. [error at 61]
int xmit_async(struct ieee802154_hw *hw, struct sk_buff *skb):
-^
Documentation/networking/ieee802154.rst:171: WARNING: Invalid C
declaration: Expected end of definition. [error at 43]
int ed(struct ieee802154_hw *hw, u8 *level):
---^
Documentation/networking/ieee802154.rst:176: WARNING: Invalid C
declaration: Expected end of definition. [error at 62]
int set_channel(struct ieee802154_hw *hw, u8 page, u8 channel):
--^
Caused by some bad c:function: prototypes. Fix them.
Acked-by: David S. Miller
Signed-off-by: Mauro Carvalho Chehab
---
Documentation/networking/ieee802154.rst | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/Documentation/networking/ieee802154.rst
b/Documentation/networking/ieee802154.rst
index 6f4bf8447a21..f27856d77c8b 100644
--- a/Documentation/networking/ieee802154.rst
+++ b/Documentation/networking/ieee802154.rst
@@ -26,7 +26,9 @@ The stack is composed of three main parts:
Socket API
==
-.. c:function:: int sd = socket(PF_IEEE802154, SOCK_DGRAM, 0);
+::
+
+int sd = socket(PF_IEEE802154, SOCK_DGRAM, 0);
The address family, socket addresses etc. are defined in the
include/net/af_ieee802154.h header or in the special header
@@ -131,12 +133,12 @@ Register PHY in the system.
Freeing registered PHY.
-.. c:function:: void ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff *skb, u8 lqi):
+.. c:function:: void ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct
sk_buff *skb, u8 lqi)
Telling 802.15.4 module there is a new received frame in the skb with
the RF Link Quality Indicator (LQI) from the hardware device.
-.. c:function:: void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb, bool ifs_handling):
+.. c:function:: void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct
sk_buff *skb, bool ifs_handling)
Telling 802.15.4 module the frame in the skb is or going to be
transmitted through the hardware device
@@ -155,25 +157,25 @@ operations structure at least::
...
};
-.. c:function:: int start(struct ieee802154_hw *hw):
+.. c:function:: int start(struct ieee802154_hw *hw)
Handler that 802.15.4 module calls for the hardware device initialization.
-.. c:function:: void stop(struct ieee802154_hw *hw):
+.. c:function:: void stop(struct ieee802154_hw *hw)
Handler that 802.15.4 module calls for the hardware device cleanup.
-.. c:function:: int xmit_async(struct ieee802154_hw *hw, struct sk_buff *skb):
+.. c:function:: int xmit_async(struct ieee802154_hw *hw, struct sk_buff *skb)
Handler that 802.15.4 module calls for each frame in the skb going to be
transmitted through the hardware device.
-.. c:function:: int ed(struct ieee802154_hw *hw, u8 *level):
+.. c:function:: int ed(struct ieee802154_hw *hw, u8 *level)
Handler that 802.15.4 module calls for Energy Detection from the hardware
device.
-.. c:function:: int set_channel(struct ieee802154_hw *hw, u8 page, u8 channel):
+.. c:function:: int set_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
Set radio for listening on specific channel of the
Re: virtiofs: WARN_ON(out_sgs + in_sgs != total_sgs)
On Sun, Oct 04, 2020 at 10:31:19AM -0400, Vivek Goyal wrote: > On Fri, Oct 02, 2020 at 10:44:37PM -0400, Qian Cai wrote: > > On Fri, 2020-10-02 at 12:28 -0400, Qian Cai wrote: > > > Running some fuzzing on virtiofs from a non-privileged user could trigger > > > a > > > warning in virtio_fs_enqueue_req(): > > > > > > WARN_ON(out_sgs + in_sgs != total_sgs); > > > > Okay, I can reproduce this after running for a few hours: > > > > out_sgs = 3, in_sgs = 2, total_sgs = 6 > > Thanks. I can also reproduce it simply by calling. > > ioctl(fd, 0x5a004000, buf); > > I think following WARN_ON() is not correct. > > WARN_ON(out_sgs + in_sgs != total_sgs) > > toal_sgs should actually be max sgs. It looks at ap->num_pages and > counts one sg for each page. And it assumes that same number of > pages will be used both for input and output. > > But there are no such guarantees. With above ioctl() call, I noticed > we are using 2 pages for input (out_sgs) and one page for output (in_sgs). > > So out_sgs=4, in_sgs=3 and total_sgs=8 and warning triggers. > > I think total sgs is actually max number of sgs and warning > should probably be. > > WARN_ON(out_sgs + in_sgs > total_sgs) > > Stefan, WDYT? It should be possible to calculate total_sgs precisely (not a maximum). Treating it as a maximum could hide bugs. Maybe sg_count_fuse_req() should count in_args/out_args[numargs - 1].size pages instead of adding ap->num_pages. Do you have the details of struct fuse_req and struct fuse_args_pages fields for the ioctl in question? Thanks, Stefan signature.asc Description: PGP signature
Re: [PATCH v2 3/5] media: mt9p031: Implement [gs]_register debug calls
Hi Laurent,
On 02.10.20 02:06, Laurent Pinchart wrote:
Hi Stefan,
On Thu, Oct 01, 2020 at 10:56:24AM +0200, Stefan Riedmüller wrote:
On 30.09.20 13:38, Laurent Pinchart wrote:
On Wed, Sep 30, 2020 at 12:51:31PM +0200, Stefan Riedmueller wrote:
From: Enrico Scholz
Implement g_register and s_register v4l2_subdev_core_ops to access
camera register directly from userspace for debug purposes.
As the name of the operations imply, this is meant for debug purpose
only. They are however prone to be abused to configure the sensor from
userspace in production, which isn't a direction we want to take.
What's your use case for this ? I'd rather drop this patch and see the
driver extended with support for more controls if needed
thanks for your feedback.
I get your point. I myself solely use these operations for debugging
purposes but I'm aware that others like to abuse them.
I thought I send it anyway since for me the DEBUG config is enough to
signalize that these operations are not to be used with a productive system.
But I'm OK with dropping this patch if you think it might send the wrong signal.
I'd rather avoid this patch due to the risk of abuse if it's OK with
you.
Yes, that's fine. I will drop it in v3.
Regards,
Stefan
Signed-off-by: Enrico Scholz
Signed-off-by: Stefan Riedmueller
---
No changes in v2
---
drivers/media/i2c/mt9p031.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index b4c042f418c1..de36025260a8 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -703,6 +703,30 @@ static int mt9p031_restore_blc(struct mt9p031 *mt9p031)
return 0;
}
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+static int mt9p031_g_register(struct v4l2_subdev *sd,
+ struct v4l2_dbg_register *reg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+ int ret;
+
+ ret = mt9p031_read(client, reg->reg);
+ if (ret < 0)
+ return ret;
+
+ reg->val = ret;
+ return 0;
+}
+
+static int mt9p031_s_register(struct v4l2_subdev *sd,
+ struct v4l2_dbg_register const *reg)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(sd);
+
+ return mt9p031_write(client, reg->reg, reg->val);
+}
+#endif
+
static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl)
{
struct mt9p031 *mt9p031 =
@@ -1000,6 +1024,10 @@ static int mt9p031_close(struct v4l2_subdev *subdev,
struct v4l2_subdev_fh *fh)
static const struct v4l2_subdev_core_ops mt9p031_subdev_core_ops = {
.s_power= mt9p031_set_power,
+#ifdef CONFIG_VIDEO_ADV_DEBUG
+ .s_register = mt9p031_s_register,
+ .g_register = mt9p031_g_register,
+#endif
};
static const struct v4l2_subdev_video_ops mt9p031_subdev_video_ops = {
Re: [PATCH v2 1/5] media: mt9p031: Add support for 8 bit and 10 bit formats
Hi Laurent,
On 02.10.20 01:53, Laurent Pinchart wrote:
Hi Stefan,
On Thu, Oct 01, 2020 at 11:07:00AM +0200, Stefan Riedmüller wrote:
On 30.09.20 13:42, Laurent Pinchart wrote:
On Wed, Sep 30, 2020 at 12:51:29PM +0200, Stefan Riedmueller wrote:
From: Christian Hemp
Aside from 12 bit monochrome or color format the sensor implicitly
supports 10 and 8 bit formats as well by simply dropping the
corresponding LSBs.
That's not how it should work though. If you set the format on
MEDIA_BUS_FMT_SGRBG8_1X8 through the pipeline for instance, you will end
up capturing the 8 LSB, not the 8 MSB.
What's your use case for this ?
I use this sensor in combination with an i.MX 6 and i.MX 6UL. When the
sensor is connected with 12 bit (or 10 bit on the i.MX 6UL) and I set
MEDIA_BUS_FMT_SGRBG8_1X8 through the pipeline the CSI interface drops the
unused 4 LSB (or 2 LSB on the i.MX 6UL) so I get the 8 MSB from my 12 bit
sensor.
Is that the PIXEL_BIT bit in CSI_CSICR1 for the i.MX6UL ? If so I think
this should be handled in the imx7-media-csi driver. You could set the
format to MEDIA_BUS_FMT_SGRBG10_1X10 on the sink pad of the CSI and to
MEDIA_BUS_FMT_SGRBG8_1X8 on the source pad to configure this. I don't
think the sensor driver should be involved, otherwise we'd have to patch
all sensor drivers. From a sensor point of view, it outputs 12-bit
Bayer, not 8-bit.
Ah, I had it wrong. What you say makes total sense. I will take another look
at it and also try to work your suggestion from below in.
Thanks,
Stefan
Now there's a caveat. When used with the i.MX6UL, I assume you connected
D[11:2] of the sensor to D[9:0] of the i.MX6UL, right ? The i.MX6UL
doesn't support 12-bit inputs, so it should accept
MEDIA_BUS_FMT_SGRBG12_1X12 on its sink pad. In this case, as D[1:0] of
the sensor are left unconnected, I think you should set data-shift to 2
and bus-width to 10 in DT on the sensor side. The MT9P031 driver should
parse that, and output MEDIA_BUS_FMT_SGRBG10_1X10 instead of
MEDIA_BUS_FMT_SGRBG12_1X12 in that case.
Does this clarify things? Maybe the description in the commit message is not
accurate enough or did I get something wrong?
Signed-off-by: Christian Hemp
[j...@pengutronix.de: simplified by dropping v4l2_colorspace handling]
Signed-off-by: Jan Luebbe
Signed-off-by: Stefan Riedmueller
---
Changes in v2:
- Use unsigned int for num_fmts and loop variable in find_datafmt
- Remove superfluous const qualifier from find_datafmt
---
drivers/media/i2c/mt9p031.c | 50 +
1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index dc23b9ed510a..2e6671ef877c 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -116,6 +116,18 @@ enum mt9p031_model {
MT9P031_MODEL_MONOCHROME,
};
+static const u32 mt9p031_color_fmts[] = {
+ MEDIA_BUS_FMT_SGRBG8_1X8,
+ MEDIA_BUS_FMT_SGRBG10_1X10,
+ MEDIA_BUS_FMT_SGRBG12_1X12,
+};
+
+static const u32 mt9p031_monochrome_fmts[] = {
+ MEDIA_BUS_FMT_Y8_1X8,
+ MEDIA_BUS_FMT_Y10_1X10,
+ MEDIA_BUS_FMT_Y12_1X12,
+};
+
struct mt9p031 {
struct v4l2_subdev subdev;
struct media_pad pad;
@@ -138,6 +150,9 @@ struct mt9p031 {
struct v4l2_ctrl *blc_auto;
struct v4l2_ctrl *blc_offset;
+ const u32 *fmts;
+ unsigned int num_fmts;
+
/* Registers cache */
u16 output_control;
u16 mode2;
@@ -148,6 +163,17 @@ static struct mt9p031 *to_mt9p031(struct v4l2_subdev *sd)
return container_of(sd, struct mt9p031, subdev);
}
+static u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code)
+{
+ unsigned int i;
+
+ for (i = 0; i < mt9p031->num_fmts; i++)
+ if (mt9p031->fmts[i] == code)
+ return mt9p031->fmts[i];
+
+ return mt9p031->fmts[mt9p031->num_fmts-1];
+}
+
static int mt9p031_read(struct i2c_client *client, u8 reg)
{
return i2c_smbus_read_word_swapped(client, reg);
@@ -476,10 +502,11 @@ static int mt9p031_enum_mbus_code(struct v4l2_subdev
*subdev,
{
struct mt9p031 *mt9p031 = to_mt9p031(subdev);
- if (code->pad || code->index)
+ if (code->pad || code->index >= mt9p031->num_fmts)
return -EINVAL;
- code->code = mt9p031->format.code;
+ code->code = mt9p031->fmts[code->index];
+
return 0;
}
@@ -573,6 +600,8 @@ static int mt9p031_set_format(struct v4l2_subdev *subdev,
__format->width = __crop->width / hratio;
__format->height = __crop->height / vratio;
+ __format->code = mt9p031_find_datafmt(mt9p031, format->format.code);
+
format->format = *__format;
return 0;
@@ -951,10 +980,7 @@ static int mt9p031_open(struct v4l2_subdev *subdev, struct
v4l2_subdev_fh *fh)
format =
Re: [PATCH v2 5/5] media: mt9p031: Fix corrupted frame after restarting stream
Hi Laurent,
On 02.10.20 02:05, Laurent Pinchart wrote:
Hi Stefan,
Thank you for the patch.
On Wed, Sep 30, 2020 at 12:51:33PM +0200, Stefan Riedmueller wrote:
From: Dirk Bender
To prevent corrupted frames after starting and stopping the sensor it's
s/it's/its/
thanks, I'll fix that.
datasheet specifies a specific pause sequence to follow:
Stopping:
Set Pause_Restart Bit -> Set Restart Bit -> Set Chip_Enable Off
Restarting:
Set Chip_Enable On -> Clear Pause_Restart Bit
The Restart Bit is cleared automatically and must not be cleared
manually as this would cause undefined behavior.
Signed-off-by: Dirk Bender
Signed-off-by: Stefan Riedmueller
---
No changes in v2
---
drivers/media/i2c/mt9p031.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index d10457361e6c..d59f66e3dcf3 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -80,6 +80,8 @@
#define MT9P031_PIXEL_CLOCK_SHIFT(n)((n) << 8)
#define MT9P031_PIXEL_CLOCK_DIVIDE(n) ((n) << 0)
#define MT9P031_FRAME_RESTART 0x0b
+#defineMT9P031_FRAME_RESTART_SET (1 << 0)
+#defineMT9P031_FRAME_PAUSE_RESTART_SET (1 << 1)
The fields are named Restart and Pause_Restart, I would drop _SET. Could
you also sort them from MSB to LSB as for the other registers ? Using
BIT() would be good too, although this could be done as an additional
patch to convert all the existing macros.
I'll do that. Also I will rename the register to MT9P031_RESTART and the
bits to MT9P031_FRAME_RESTART and MT9P031_FRAME_PAUSE_RESTART.
#define MT9P031_SHUTTER_DELAY 0x0c
#define MT9P031_RST 0x0d
#define MT9P031_RST_ENABLE 1
@@ -483,9 +485,25 @@ static int mt9p031_set_params(struct mt9p031 *mt9p031)
static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable)
{
struct mt9p031 *mt9p031 = to_mt9p031(subdev);
+ struct i2c_client *client = v4l2_get_subdevdata(subdev);
+ int val;
int ret;
if (!enable) {
+ val = mt9p031_read(client, MT9P031_FRAME_RESTART);
Do you need to read the register ? Can't you write
MT9P031_FRAME_PAUSE_RESTART_SET and then MT9P031_FRAME_PAUSE_RESTART_SET
| MT9P031_FRAME_RESTART_SET ? And actually, can't we just write both
bits in one go, do we need two writes ?
I think you're right we don't necessarily need to read the registers. The
only other bit is not used by the driver.
But I think we do need two separate writes, at least that is what the
datasheet states.
So I would drop the read but keep both write, ok?
+
+ /* enable pause restart */
+ val |= MT9P031_FRAME_PAUSE_RESTART_SET;
+ ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val);
+ if (ret < 0)
+ return ret;
+
+ /* enable restart + keep pause restart set */
+ val |= MT9P031_FRAME_RESTART_SET;
+ ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val);
+ if (ret < 0)
+ return ret;
+
/* Stop sensor readout */
ret = mt9p031_set_output_control(mt9p031,
MT9P031_OUTPUT_CONTROL_CEN, 0);
@@ -505,6 +523,13 @@ static int mt9p031_s_stream(struct v4l2_subdev *subdev,
int enable)
if (ret < 0)
return ret;
+ val = mt9p031_read(client, MT9P031_FRAME_RESTART);
+ /* disable reset + pause restart */
+ val &= ~MT9P031_FRAME_PAUSE_RESTART_SET;
Same here, I think you can simply write MT9P031_FRAME_PAUSE_RESTART_SET.
I'll drop the read here as well. But I need to make sure, that the Restart
Bit is not cleared manually here.
Regards,
Stefan
+ ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val);
+ if (ret < 0)
+ return ret;
+
return mt9p031_pll_enable(mt9p031);
}
Re: [PATCH v2 4/5] media: mt9p031: Make pixel clock polarity configurable by DT
Hi Sakari, On 01.10.20 18:11, Sakari Ailus wrote: On Thu, Oct 01, 2020 at 07:10:31PM +0300, Sakari Ailus wrote: @@ -1079,6 +1094,9 @@ mt9p031_get_pdata(struct i2c_client *client) of_property_read_u32(np, "input-clock-frequency", >ext_freq); of_property_read_u32(np, "pixel-clock-frequency", >target_freq); + pdata->pixclk_pol = !!(endpoint.bus.parallel.flags & + V4L2_MBUS_PCLK_SAMPLE_RISING); Could you document this in DT bindings? And the default, too. Please make it a separate patch. Sure, I'll send a separate patch for the DT bindings. Regards, Stefan
Re: [PATCH][next] mtd: rawnand: Replace one-element array with flexible-array member
On 2020-10-01 10:12, Miquel Raynal wrote: > Hi Jann, > > Jann Horn wrote on Thu, 1 Oct 2020 00:32:24 +0200: > >> On Wed, Sep 30, 2020 at 11:30 PM Gustavo A. R. Silva >> wrote: >> > On Wed, Sep 30, 2020 at 11:10:43PM +0200, Jann Horn wrote: >> > > On Wed, Sep 30, 2020 at 11:02 PM Gustavo A. R. Silva >> > > wrote: >> > > > There is a regular need in the kernel to provide a way to declare >> > > > having >> > > > a dynamically sized set of trailing elements in a structure. Kernel >> > > > code >> > > > should always use “flexible array members”[1] for these cases. The >> > > > older >> > > > style of one-element or zero-length arrays should no longer be used[2]. >> > > >> > > But this is not such a case, right? Isn't this a true fixed-size >> > > array? It sounds like you're just changing it because it >> > > pattern-matched on "array of length 1 at the end of a struct". >> > >> > Yeah; I should have changed that 'dynamically' part of the text above >> > a bit. However, as I commented in the text below, in the case that more >> > CS IDs are needed (let's wait for the maintainers to comment on this...) >> > in the future, this change makes the code more maintainable, as for >> > the allocation part, the developer would only have to update the CS_N >> > macro to the number of CS IDs that are needed. >> >> But in that case, shouldn't you change it to "int cs[CS_N]" and get >> rid of the struct_size() stuff? > > I do agree with Jann, I think it's best to consider this a fixed-size > array for now. If we ever want to extend the number of supported CS, > there is much more rework involved anyway. I agree, too, just assume this is a fixed-size array of 1 element. In fact, I am not aware of a design which needs multiple chip selects. -- Stefan
