* Re: [PATCH v2 1/3] dt-bindings: PCI: brcmstb: Add two optional props
From: Konstantin Ryabitsev @ 2023-04-21 19:07 UTC (permalink / raw)
To: Rob Herring, Florian Fainelli
Cc: Jim Quinlan, Krzysztof Kozlowski, linux-pci,
Nicolas Saenz Julienne, Bjorn Helgaas, Lorenzo Pieralisi,
Cyril Brulebois, Phil Elwell, bcm-kernel-feedback-list,
james.quinlan, Lorenzo Pieralisi, Krzysztof Wilczyński,
Krzysztof Kozlowski,
moderated list:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20230418183536.GA2087834-robh@kernel.org>
April 18, 2023 2:35 PM, "Rob Herring" <robh@kernel.org> wrote:
>> Some trees like the networking
>> tree do merge commits of patch sets where the cover letter is used as part
>> of the merge commit message. Other maintainers don't, and some want the
>> change log after the '---' and some do not.
>
> I'm not aware of anyone except for DRM wanting the changelog in the
> final commits, but that's really a different issue.
I don't think anyone wants changelogs in actual final commits, they usually go under "---" in patch submissions.
> I'm pretty sure no one will complain about a changelog in the patches. I
> guess you just have to duplicate it if you think it should be in both.
> b4 could be taught to do that I suppose. IMO, the cover letter should
> have a higher level changelog than the individual patches.
b4 doesn't really need to manage per-patch changelogs -- they should just go under "---" in the commit. When you send the series either via "b4 send" or via git-send-email, the changelogs will be properly included in the message, but they won't make it into the tree after the maintainer runs "git am", because git will drop anything under the first "---" in the commit message.
The cover letter changelog is supposed to be higher level than individual patch changelogs, so I don't think it makes sense for b4 to collect them from individual patches.
-K
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 20/22] net: dsa: mt7530: force link-down on MACs before reset on MT7530
From: Daniel Golle @ 2023-04-21 19:06 UTC (permalink / raw)
To: Arınç ÜNAL
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <7982894a-029c-585a-9ab5-3a6295c6abaa@arinc9.com>
On Fri, Apr 21, 2023 at 09:47:16PM +0300, Arınç ÜNAL wrote:
> On 21.04.2023 21:42, Daniel Golle wrote:
> > On Fri, Apr 21, 2023 at 05:36:46PM +0300, arinc9.unal@gmail.com wrote:
> > > From: Arınç ÜNAL <arinc.unal@arinc9.com>
> > >
> > > Force link-down on all MACs before internal reset. Let's follow suit commit
> > > 728c2af6ad8c ("net: mt7531: ensure all MACs are powered down before
> > > reset").
> > >
> > > Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> > > ---
> > > drivers/net/dsa/mt7530.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > > index ac1e3c58aaac..8ece3d0d820c 100644
> > > --- a/drivers/net/dsa/mt7530.c
> > > +++ b/drivers/net/dsa/mt7530.c
> > > @@ -2203,6 +2203,10 @@ mt7530_setup(struct dsa_switch *ds)
> > > return -EINVAL;
> > > }
> > > + /* Force link-down on all MACs before internal reset */
> > > + for (i = 0; i < MT7530_NUM_PORTS; i++)
> > > + mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
> > > +
> >
> > Moving this part to mt753x_setup just before calling priv->info->sw_setup(ds);
> > is probably better. Though it isn't documented I assume that the requirement
> > to have the ports in force-link-down may also apply to MT7988, and for sure
> > it doesn't do any harm.
> >
> > Hence I suggest to squash this change:
> > diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> > index a2cb7e296165e..998c4e8930cd3 100644
> > --- a/drivers/net/dsa/mt7530.c
> > +++ b/drivers/net/dsa/mt7530.c
> > @@ -2203,10 +2203,6 @@ mt7530_setup(struct dsa_switch *ds)
> > return -EINVAL;
> > }
> > - /* Force link-down on all MACs before internal reset */
> > - for (i = 0; i < MT7530_NUM_PORTS; i++)
> > - mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
> > -
> > /* Reset the switch through internal reset */
> > mt7530_write(priv, MT7530_SYS_CTRL,
> > SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
> > @@ -2423,10 +2419,6 @@ mt7531_setup(struct dsa_switch *ds)
> > dev_info(priv->dev, "found MT7531BE\n");
> > }
> > - /* all MACs must be forced link-down before sw reset */
> > - for (i = 0; i < MT7530_NUM_PORTS; i++)
> > - mt7530_write(priv, MT7530_PMCR_P(i), MT7531_FORCE_LNK);
> > -
> > /* Reset the switch through internal reset */
> > mt7530_write(priv, MT7530_SYS_CTRL,
> > SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
> > @@ -2907,6 +2899,10 @@ mt753x_setup(struct dsa_switch *ds)
> > priv->pcs[i].port = i;
> > }
> > + /* Force link-down on all MACs before setup */
> > + for (i = 0; i < MT7530_NUM_PORTS; i++)
> > + mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
>
> MT7531 has got a different bit on the register for this, MT7531_FORCE_LNK.
> Are you sure PMCR_FORCE_LNK would work for MT7531 too?
No, I had overlooked that. As the effects of not doing the
force-link-down before the reset are subtle and depend on the
link-partners I may not have cought them in my tests.
>
> Arınç
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: misc: esm: Add ESM support for TI K3 devices
From: Rob Herring @ 2023-04-21 19:06 UTC (permalink / raw)
To: Neha Malcom Francis
Cc: krzysztof.kozlowski+dt, devicetree, linux-kernel,
linux-arm-kernel, nm, vigneshr, u-kumar1
In-Reply-To: <20230419092559.673869-2-n-francis@ti.com>
On Wed, Apr 19, 2023 at 02:55:57PM +0530, Neha Malcom Francis wrote:
> Document the binding for TI K3 ESM (Error Signaling Module) block.
>
> Signed-off-by: Neha Malcom Francis <n-francis@ti.com>
> ---
> .../bindings/hwmon/ti,j721e-esm.yaml | 55 +++++++++++++++++++
> 1 file changed, 55 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/ti,j721e-esm.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,j721e-esm.yaml b/Documentation/devicetree/bindings/hwmon/ti,j721e-esm.yaml
> new file mode 100644
> index 000000000000..7b23ac7cb3ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/ti,j721e-esm.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 Texas Instruments Incorporated
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/ti,j721e-esm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments K3 ESM
> +
> +maintainers:
> + - Neha Malcom Francis <n-francis@ti.com>
> +
> +description:
> + The ESM (Error Signaling Module) is an IP block on TI K3 devices
> + that allows handling of safety events somewhat similar to what interrupt
> + controller would do. The safety signals have their separate paths within
> + the SoC, and they are handled by the ESM, which routes them to the proper
> + destination, which can be system reset, interrupt controller, etc. In the
> + simplest configuration the signals are just routed to reset the SoC.
> +
> +properties:
> + compatible:
> + const: ti,j721e-esm
> +
> + reg:
> + items:
> + - description: the ESM register set
That's kind of obvious... Just 'maxItems: 1' is sufficient here.
> +
> + ti,esm-pins:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description:
> + integer array of ESM interrupt pins to route to external event pin
> + which can be used to reset the SoC.
> + minItems: 1
> + maxItems: 255
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - ti,esm-pins
> +
> +examples:
> + - |
> + cbass_main {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + esm@700000 {
> + compatible = "ti,j721e-esm";
> + reg = <0x0 0x700000 0x0 0x1000>;
> + ti,esm-pins = <344>, <345>;
> + };
> + };
> --
> 2.34.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 08/22] net: dsa: mt7530: change p{5,6}_interface to p{5,6}_configured
From: Daniel Golle @ 2023-04-21 19:03 UTC (permalink / raw)
To: Arınç ÜNAL
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <235c80fc-3f1b-a9c9-6364-6f50ee45b21b@arinc9.com>
On Fri, Apr 21, 2023 at 09:25:39PM +0300, Arınç ÜNAL wrote:
>
>
> On 21.04.2023 21:20, Arınç ÜNAL wrote:
> > On 21.04.2023 21:17, Arınç ÜNAL wrote:
> > > On 21.04.2023 20:28, Daniel Golle wrote:
> > > > On Fri, Apr 21, 2023 at 05:36:34PM +0300, arinc9.unal@gmail.com wrote:
> > > > > From: Arınç ÜNAL <arinc.unal@arinc9.com>
> > > > >
> > > > > The idea of p5_interface and p6_interface pointers is to prevent
> > > > > mt753x_mac_config() from running twice for MT7531, as it's
> > > > > already run with
> > > > > mt753x_cpu_port_enable() from mt7531_setup_common(), if the
> > > > > port is used as
> > > > > a CPU port.
> > > > >
> > > > > Change p5_interface and p6_interface to p5_configured and
> > > > > p6_configured.
> > > > > Make them boolean.
> > > > >
> > > > > Do not set them for any other reason.
> > > > >
> > > > > The priv->p5_intf_sel check is useless as in this code path,
> > > > > it will always
> > > > > be P5_INTF_SEL_GMAC5.
> > > > >
> > > > > There was also no need to set priv->p5_interface and
> > > > > priv->p6_interface to
> > > > > PHY_INTERFACE_MODE_NA on mt7530_setup() and mt7531_setup()
> > > > > as they would
> > > > > already be set to that when "priv" is allocated. The
> > > > > pointers were of the
> > > > > phy_interface_t enumeration type, and the first element of the enum is
> > > > > PHY_INTERFACE_MODE_NA. There was nothing in between that
> > > > > would change this
> > > > > beforehand.
> > > > >
> > > > > Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> > > > > Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> > > >
> > > > NACK. This assumes that a user port is configured exactly once.
> > > > However, interface mode may change because of mode-changing PHYs (e.g.
> > > > often using Cisco SGMII for 10M/100M/1000M but using 2500Base-X for
> > > > 2500M, ie. depending on actual link speed).
> > > >
> > > > Also when using SFP modules (which can be hotplugged) the interface
> > > > mode may change after initially setting up the driver, e.g. when SFP
> > > > driver is loaded or a module is plugged or replaced.
> > >
> > > I'm not sure I understand. pX_configured would be set to true only
> > > when the port is used as a CPU port. mt753x_mac_config() should run
> > > for user or DSA ports more than once, if needed.
> >
> > Looking at this again, once pX_interface is true, the check will prevent
> > even user or DSA ports to be configured again. What about setting
> > pX_interface to false after mt753x_mac_config() is run?
>
> On a third thought, pX_interface will never be true for the port if it's a
> user or DSA port so this should not be a problem at all.
I also followed the individual codepaths and conclude that you are
right. I've also tested this by now on several boards with MT753x,
incl. BPi-R3 and SerDes interface mode switching is anyway handled in
the PCS driver (I should have rembered that...)
Hence
Acked-by: Daniel Golle <daniel@makrotopia.org>
>
> Arınç
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: dts: s5pv210: remove empty camera pinctrl configuration
From: Krzysztof Kozlowski @ 2023-04-21 19:02 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Alim Akhtar, devicetree,
linux-arm-kernel, linux-samsung-soc, linux-kernel
Cc: Krzysztof Kozlowski
The camera's pinctrl configuration is simply empty and not effective.
Remove it to fix dtbs_check warnings like:
s5pv210-torbreck.dtb: camera@fa600000: pinctrl-0: True is not of type 'array'
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
arch/arm/boot/dts/s5pv210.dtsi | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm/boot/dts/s5pv210.dtsi b/arch/arm/boot/dts/s5pv210.dtsi
index d9436bbf77c8..faa3682ab5dd 100644
--- a/arch/arm/boot/dts/s5pv210.dtsi
+++ b/arch/arm/boot/dts/s5pv210.dtsi
@@ -549,8 +549,6 @@ i2c1: i2c@fab00000 {
camera: camera@fa600000 {
compatible = "samsung,fimc";
- pinctrl-names = "default";
- pinctrl-0 = <>;
clocks = <&clocks SCLK_CAM0>, <&clocks SCLK_CAM1>;
clock-names = "sclk_cam0", "sclk_cam1";
#address-cells = <1>;
--
2.34.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH net-next 21/22] net: dsa: mt7530: get rid of useless error returns on phylink code path
From: Daniel Golle @ 2023-04-21 18:58 UTC (permalink / raw)
To: arinc9.unal
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King,
Arınç ÜNAL, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20230421143648.87889-22-arinc.unal@arinc9.com>
On Fri, Apr 21, 2023 at 05:36:47PM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>
> Remove error returns on the cases where they are already handled with the
> function the mac_port_get_caps member points to.
>
> mt7531_mac_config() is also called from mt7531_cpu_port_config() outside of
> phylink but the port and interface modes are already handled there.
>
> Change the functions and the mac_port_config function pointer to void now
> that there're no error returns anymore.
>
> Remove mt753x_is_mac_port() that used to help the said error returns.
>
> On mt7531_mac_config(), switch to if statements to simplify the code.
>
> Remove internal phy cases from mt753x_phylink_mac_config() as there is no
> configuration to be done for them. There's also no need to check the
> interface mode as that's already handled with the function the
> mac_port_get_caps member points to.
>
> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Acked-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Daniel Golle <daniel@makrotopia.org>
(on BPi-R3 MT7986A+MT7531AE, BPi-R64 MT7622+MT7531BE and MT7988A rfb)
> ---
> drivers/net/dsa/mt7530.c | 81 ++++++++--------------------------------
> drivers/net/dsa/mt7530.h | 2 +-
> 2 files changed, 17 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index 8ece3d0d820c..3d19e06061cb 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2556,7 +2556,7 @@ static void mt7988_mac_port_get_caps(struct dsa_switch *ds, int port,
> }
> }
>
> -static int
> +static void
> mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> phy_interface_t interface)
> {
> @@ -2567,22 +2567,14 @@ mt7530_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> } else if (port == 6) {
> mt7530_setup_port6(priv->ds, interface);
> }
> -
> - return 0;
> }
>
> -static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
> - phy_interface_t interface,
> - struct phy_device *phydev)
> +static void mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
> + phy_interface_t interface,
> + struct phy_device *phydev)
> {
> u32 val;
>
> - if (priv->p5_sgmii) {
> - dev_err(priv->dev, "RGMII mode is not available for port %d\n",
> - port);
> - return -EINVAL;
> - }
> -
> val = mt7530_read(priv, MT7531_CLKGEN_CTRL);
> val |= GP_CLK_EN;
> val &= ~GP_MODE_MASK;
> @@ -2610,20 +2602,14 @@ static int mt7531_rgmii_setup(struct mt7530_priv *priv, u32 port,
> case PHY_INTERFACE_MODE_RGMII_ID:
> break;
> default:
> - return -EINVAL;
> + break;
> }
> }
> - mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
>
> - return 0;
> -}
> -
> -static bool mt753x_is_mac_port(u32 port)
> -{
> - return (port == 5 || port == 6);
> + mt7530_write(priv, MT7531_CLKGEN_CTRL, val);
> }
>
> -static int
> +static void
> mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> phy_interface_t interface)
> {
> @@ -2631,42 +2617,21 @@ mt7531_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> struct phy_device *phydev;
> struct dsa_port *dp;
>
> - if (!mt753x_is_mac_port(port)) {
> - dev_err(priv->dev, "port %d is not a MAC port\n", port);
> - return -EINVAL;
> - }
> -
> - switch (interface) {
> - case PHY_INTERFACE_MODE_RGMII:
> - case PHY_INTERFACE_MODE_RGMII_ID:
> - case PHY_INTERFACE_MODE_RGMII_RXID:
> - case PHY_INTERFACE_MODE_RGMII_TXID:
> + if (phy_interface_mode_is_rgmii(interface)) {
> dp = dsa_to_port(ds, port);
> phydev = dp->slave->phydev;
> - return mt7531_rgmii_setup(priv, port, interface, phydev);
> - case PHY_INTERFACE_MODE_SGMII:
> - case PHY_INTERFACE_MODE_NA:
> - case PHY_INTERFACE_MODE_1000BASEX:
> - case PHY_INTERFACE_MODE_2500BASEX:
> - /* handled in SGMII PCS driver */
> - return 0;
> - default:
> - return -EINVAL;
> + mt7531_rgmii_setup(priv, port, interface, phydev);
> }
> -
> - return -EINVAL;
> }
>
> -static int
> +static void
> mt753x_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> const struct phylink_link_state *state)
> {
> struct mt7530_priv *priv = ds->priv;
>
> - if (!priv->info->mac_port_config)
> - return 0;
> -
> - return priv->info->mac_port_config(ds, port, mode, state->interface);
> + if (priv->info->mac_port_config)
> + priv->info->mac_port_config(ds, port, mode, state->interface);
> }
>
> static struct phylink_pcs *
> @@ -2695,30 +2660,18 @@ mt753x_phylink_mac_config(struct dsa_switch *ds, int port, unsigned int mode,
> u32 mcr_cur, mcr_new;
>
> switch (port) {
> - case 0 ... 4: /* Internal phy */
> - if (state->interface != PHY_INTERFACE_MODE_GMII &&
> - state->interface != PHY_INTERFACE_MODE_INTERNAL)
> - goto unsupported;
> - break;
> case 5: /* Port 5, can be used as a CPU port. */
> if (priv->p5_configured)
> break;
>
> - if (mt753x_mac_config(ds, port, mode, state) < 0)
> - goto unsupported;
> + mt753x_mac_config(ds, port, mode, state);
> break;
> case 6: /* Port 6, can be used as a CPU port. */
> if (priv->p6_configured)
> break;
>
> - if (mt753x_mac_config(ds, port, mode, state) < 0)
> - goto unsupported;
> + mt753x_mac_config(ds, port, mode, state);
> break;
> - default:
> -unsupported:
> - dev_err(ds->dev, "%s: unsupported %s port: %i\n",
> - __func__, phy_modes(state->interface), port);
> - return;
> }
>
> mcr_cur = mt7530_read(priv, MT7530_PMCR_P(port));
> @@ -2811,7 +2764,6 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
> struct mt7530_priv *priv = ds->priv;
> phy_interface_t interface;
> int speed;
> - int ret;
>
> switch (port) {
> case 5:
> @@ -2836,9 +2788,8 @@ mt7531_cpu_port_config(struct dsa_switch *ds, int port)
> else
> speed = SPEED_1000;
>
> - ret = mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
> - if (ret)
> - return ret;
> + mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
> +
> mt7530_write(priv, MT7530_PMCR_P(port),
> PMCR_CPU_PORT_SETTING(priv->id));
> mt753x_phylink_pcs_link_up(&priv->pcs[port].pcs, MLO_AN_FIXED,
> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
> index cad9115de22b..ee2b3d2d6258 100644
> --- a/drivers/net/dsa/mt7530.h
> +++ b/drivers/net/dsa/mt7530.h
> @@ -722,7 +722,7 @@ struct mt753x_info {
> void (*mac_port_validate)(struct dsa_switch *ds, int port,
> phy_interface_t interface,
> unsigned long *supported);
> - int (*mac_port_config)(struct dsa_switch *ds, int port,
> + void (*mac_port_config)(struct dsa_switch *ds, int port,
> unsigned int mode,
> phy_interface_t interface);
> };
> --
> 2.37.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 20/22] net: dsa: mt7530: force link-down on MACs before reset on MT7530
From: Arınç ÜNAL @ 2023-04-21 18:47 UTC (permalink / raw)
To: Daniel Golle
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <ZELZAd4O9SyHLkwn@makrotopia.org>
On 21.04.2023 21:42, Daniel Golle wrote:
> On Fri, Apr 21, 2023 at 05:36:46PM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Force link-down on all MACs before internal reset. Let's follow suit commit
>> 728c2af6ad8c ("net: mt7531: ensure all MACs are powered down before
>> reset").
>>
>> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
>> drivers/net/dsa/mt7530.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index ac1e3c58aaac..8ece3d0d820c 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -2203,6 +2203,10 @@ mt7530_setup(struct dsa_switch *ds)
>> return -EINVAL;
>> }
>>
>> + /* Force link-down on all MACs before internal reset */
>> + for (i = 0; i < MT7530_NUM_PORTS; i++)
>> + mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
>> +
>
> Moving this part to mt753x_setup just before calling priv->info->sw_setup(ds);
> is probably better. Though it isn't documented I assume that the requirement
> to have the ports in force-link-down may also apply to MT7988, and for sure
> it doesn't do any harm.
>
> Hence I suggest to squash this change:
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index a2cb7e296165e..998c4e8930cd3 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2203,10 +2203,6 @@ mt7530_setup(struct dsa_switch *ds)
> return -EINVAL;
> }
>
> - /* Force link-down on all MACs before internal reset */
> - for (i = 0; i < MT7530_NUM_PORTS; i++)
> - mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
> -
> /* Reset the switch through internal reset */
> mt7530_write(priv, MT7530_SYS_CTRL,
> SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
> @@ -2423,10 +2419,6 @@ mt7531_setup(struct dsa_switch *ds)
> dev_info(priv->dev, "found MT7531BE\n");
> }
>
> - /* all MACs must be forced link-down before sw reset */
> - for (i = 0; i < MT7530_NUM_PORTS; i++)
> - mt7530_write(priv, MT7530_PMCR_P(i), MT7531_FORCE_LNK);
> -
> /* Reset the switch through internal reset */
> mt7530_write(priv, MT7530_SYS_CTRL,
> SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
> @@ -2907,6 +2899,10 @@ mt753x_setup(struct dsa_switch *ds)
> priv->pcs[i].port = i;
> }
>
> + /* Force link-down on all MACs before setup */
> + for (i = 0; i < MT7530_NUM_PORTS; i++)
> + mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
MT7531 has got a different bit on the register for this,
MT7531_FORCE_LNK. Are you sure PMCR_FORCE_LNK would work for MT7531 too?
Arınç
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 20/22] net: dsa: mt7530: force link-down on MACs before reset on MT7530
From: Daniel Golle @ 2023-04-21 18:42 UTC (permalink / raw)
To: arinc9.unal
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King,
Arınç ÜNAL, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20230421143648.87889-21-arinc.unal@arinc9.com>
On Fri, Apr 21, 2023 at 05:36:46PM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>
> Force link-down on all MACs before internal reset. Let's follow suit commit
> 728c2af6ad8c ("net: mt7531: ensure all MACs are powered down before
> reset").
>
> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> ---
> drivers/net/dsa/mt7530.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index ac1e3c58aaac..8ece3d0d820c 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2203,6 +2203,10 @@ mt7530_setup(struct dsa_switch *ds)
> return -EINVAL;
> }
>
> + /* Force link-down on all MACs before internal reset */
> + for (i = 0; i < MT7530_NUM_PORTS; i++)
> + mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
> +
Moving this part to mt753x_setup just before calling priv->info->sw_setup(ds);
is probably better. Though it isn't documented I assume that the requirement
to have the ports in force-link-down may also apply to MT7988, and for sure
it doesn't do any harm.
Hence I suggest to squash this change:
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index a2cb7e296165e..998c4e8930cd3 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2203,10 +2203,6 @@ mt7530_setup(struct dsa_switch *ds)
return -EINVAL;
}
- /* Force link-down on all MACs before internal reset */
- for (i = 0; i < MT7530_NUM_PORTS; i++)
- mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
-
/* Reset the switch through internal reset */
mt7530_write(priv, MT7530_SYS_CTRL,
SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
@@ -2423,10 +2419,6 @@ mt7531_setup(struct dsa_switch *ds)
dev_info(priv->dev, "found MT7531BE\n");
}
- /* all MACs must be forced link-down before sw reset */
- for (i = 0; i < MT7530_NUM_PORTS; i++)
- mt7530_write(priv, MT7530_PMCR_P(i), MT7531_FORCE_LNK);
-
/* Reset the switch through internal reset */
mt7530_write(priv, MT7530_SYS_CTRL,
SYS_CTRL_PHY_RST | SYS_CTRL_SW_RST |
@@ -2907,6 +2899,10 @@ mt753x_setup(struct dsa_switch *ds)
priv->pcs[i].port = i;
}
+ /* Force link-down on all MACs before setup */
+ for (i = 0; i < MT7530_NUM_PORTS; i++)
+ mt7530_write(priv, MT7530_PMCR_P(i), PMCR_FORCE_LNK);
+
ret = priv->info->sw_setup(ds);
if (ret)
return ret;
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC PATCH net-next 19/22] net: dsa: mt7530: set interrupt register only for MT7530
From: Daniel Golle @ 2023-04-21 18:32 UTC (permalink / raw)
To: arinc9.unal
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King,
Arınç ÜNAL, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <20230421143648.87889-20-arinc.unal@arinc9.com>
On Fri, Apr 21, 2023 at 05:36:45PM +0300, arinc9.unal@gmail.com wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>
> Setting this register related to interrupts is only needed for the MT7530
> switch. Make an exclusive check to ensure this.
>
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Acked-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Daniel Golle <daniel@makrotopia.org>
(on MT7988 which is the relevant hardware regarding this change,
interrupts still work fine)
> ---
> drivers/net/dsa/mt7530.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
> index a66a762cb5db..ac1e3c58aaac 100644
> --- a/drivers/net/dsa/mt7530.c
> +++ b/drivers/net/dsa/mt7530.c
> @@ -2034,7 +2034,7 @@ mt7530_setup_irq(struct mt7530_priv *priv)
> }
>
> /* This register must be set for MT7530 to properly fire interrupts */
> - if (priv->id != ID_MT7531)
> + if (priv->id == ID_MT7530 || priv->id == ID_MT7621)
> mt7530_set(priv, MT7530_TOP_SIG_CTRL, TOP_SIG_CTRL_NORMAL);
>
> ret = request_threaded_irq(priv->irq, NULL, mt7530_irq_thread_fn,
> --
> 2.37.2
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 03/22] net: dsa: mt7530: properly support MT7531AE and MT7531BE
From: Arınç ÜNAL @ 2023-04-21 18:29 UTC (permalink / raw)
To: Daniel Golle
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <ZEKqH2oELsJKANkh@makrotopia.org>
On 21.04.2023 18:22, Daniel Golle wrote:
> On Fri, Apr 21, 2023 at 05:36:29PM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Introduce the p5_sgmii pointer to store the information for whether port 5
>> has got SGMII or not. Print "found MT7531AE" if it's got it, print "found
>> MT7531BE" if it hasn't.
>>
>> Move the comment about MT7531AE and MT7531BE to mt7531_setup(), where the
>> switch is identified.
>>
>> Get rid of mt7531_dual_sgmii_supported() now that priv->p5_sgmii stores the
>> information. Address the code where mt7531_dual_sgmii_supported() is used.
>>
>> Get rid of mt7531_is_rgmii_port() which just prints the opposite of
>> priv->p5_sgmii.
>>
>> Remove P5_INTF_SEL_GMAC5_SGMII. The p5_interface_select enum is supposed to
>> represent the mode that port 5 is used in, not the hardware information of
>> port 5. Set p5_intf_sel to P5_INTF_SEL_GMAC5 instead, if port 5 is not
>> dsa_is_unused_port().
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
>> drivers/net/dsa/mt7530-mdio.c | 7 ++---
>> drivers/net/dsa/mt7530.c | 49 +++++++++++++++--------------------
>> drivers/net/dsa/mt7530.h | 6 +++--
>> 3 files changed, 27 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
>> index 088533663b83..fa3ee85a99c1 100644
>> --- a/drivers/net/dsa/mt7530-mdio.c
>> +++ b/drivers/net/dsa/mt7530-mdio.c
>> @@ -81,17 +81,14 @@ static const struct regmap_bus mt7530_regmap_bus = {
>> };
>>
>> static int
>> -mt7531_create_sgmii(struct mt7530_priv *priv, bool dual_sgmii)
>> +mt7531_create_sgmii(struct mt7530_priv *priv)
>> {
>> struct regmap_config *mt7531_pcs_config[2] = {};
>> struct phylink_pcs *pcs;
>> struct regmap *regmap;
>> int i, ret = 0;
>>
>> - /* MT7531AE has two SGMII units for port 5 and port 6
>> - * MT7531BE has only one SGMII unit for port 6
>> - */
>> - for (i = dual_sgmii ? 0 : 1; i < 2; i++) {
>> + for (i = priv->p5_sgmii ? 0 : 1; i < 2; i++) {
>> mt7531_pcs_config[i] = devm_kzalloc(priv->dev,
>> sizeof(struct regmap_config),
>> GFP_KERNEL);
>> diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
>> index c680873819b0..edc34be745b2 100644
>> --- a/drivers/net/dsa/mt7530.c
>> +++ b/drivers/net/dsa/mt7530.c
>> @@ -473,15 +473,6 @@ mt7530_pad_clk_setup(struct dsa_switch *ds, phy_interface_t interface)
>> return 0;
>> }
>>
>> -static bool mt7531_dual_sgmii_supported(struct mt7530_priv *priv)
>> -{
>> - u32 val;
>> -
>> - val = mt7530_read(priv, MT7531_TOP_SIG_SR);
>> -
>> - return (val & PAD_DUAL_SGMII_EN) != 0;
>> -}
>> -
>> static int
>> mt7531_pad_setup(struct dsa_switch *ds, phy_interface_t interface)
>> {
>> @@ -496,7 +487,7 @@ mt7531_pll_setup(struct mt7530_priv *priv)
>> u32 xtal;
>> u32 val;
>>
>> - if (mt7531_dual_sgmii_supported(priv))
>> + if (priv->p5_sgmii)
>> return;
>>
>> val = mt7530_read(priv, MT7531_CREV);
>> @@ -907,8 +898,6 @@ static const char *p5_intf_modes(unsigned int p5_interface)
>> return "PHY P4";
>> case P5_INTF_SEL_GMAC5:
>> return "GMAC5";
>> - case P5_INTF_SEL_GMAC5_SGMII:
>> - return "GMAC5_SGMII";
>> default:
>> return "unknown";
>> }
>> @@ -2440,6 +2429,18 @@ mt7531_setup(struct dsa_switch *ds)
>> return -ENODEV;
>> }
>>
>> + /* MT7531AE has got two SGMII units. One for port 5, one for port 6.
>> + * MT7531BE has got only one SGMII unit which is for port 6.
>> + */
>> + val = mt7530_read(priv, MT7531_TOP_SIG_SR);
>> +
>> + if ((val & PAD_DUAL_SGMII_EN) != 0) {
>> + priv->p5_sgmii = true;
>> + dev_info(priv->dev, "found MT7531AE\n");
>> + } else {
>> + dev_info(priv->dev, "found MT7531BE\n");
>
> I don't think dev_info is useful here for regular users.
> If you really want this output, use dev_dbg to reduce log pollution.
> Imho completely removing the else branch and only silently
> setting priv->p5_sgmii is sufficient, as users can also turn on
> dyndbg for drivers/net/pcs/pcs-mtk-lynxi.c and will then be informed
> about the created SGMII/1000Base-X/2500Base-X PCS instances.
Sounds good, I'll drop it.
Arınç
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 02/22] net: dsa: mt7530: use p5_interface_select as data type for p5_intf_sel
From: Arınç ÜNAL @ 2023-04-21 18:27 UTC (permalink / raw)
To: Daniel Golle
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <ZEK4gVx-WQv0j2cR@makrotopia.org>
On 21.04.2023 19:23, Daniel Golle wrote:
> On Fri, Apr 21, 2023 at 05:36:28PM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> Use the p5_interface_select enumeration as the data type for the
>> p5_intf_sel field. This ensures p5_intf_sel can only take the values
>> defined in the p5_interface_select enumeration.
>>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> ---
>> drivers/net/dsa/mt7530.h | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
>> index 845f5dd16d83..703f8a528317 100644
>> --- a/drivers/net/dsa/mt7530.h
>> +++ b/drivers/net/dsa/mt7530.h
>> @@ -674,13 +674,13 @@ struct mt7530_port {
>> };
>>
>> /* Port 5 interface select definitions */
>> -enum p5_interface_select {
>> - P5_DISABLED = 0,
>> +typedef enum {
>
> We usually avoid adding typedef in kernel code. If the purpose is
> just to be more verbose in the struct definition, you can as well
> also just use 'enum p5_interface_select as type in the struct.
>
>> + P5_DISABLED,
>> P5_INTF_SEL_PHY_P0,
>> P5_INTF_SEL_PHY_P4,
>> P5_INTF_SEL_GMAC5,
>> P5_INTF_SEL_GMAC5_SGMII,
>> -};
>> +} p5_interface_select;
>>
>> struct mt7530_priv;
>>
>> @@ -768,7 +768,7 @@ struct mt7530_priv {
>> bool mcm;
>> phy_interface_t p6_interface;
>> phy_interface_t p5_interface;
>> - unsigned int p5_intf_sel;
>> + p5_interface_select p5_intf_sel;
>
> enum p5_interface_select p5_intf_sel;
Will do, thanks.
Arınç
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 08/22] net: dsa: mt7530: change p{5,6}_interface to p{5,6}_configured
From: Arınç ÜNAL @ 2023-04-21 18:25 UTC (permalink / raw)
To: Daniel Golle
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <f1c38c13-a1f6-93d8-90ae-4ea3f7e06dc2@arinc9.com>
On 21.04.2023 21:20, Arınç ÜNAL wrote:
> On 21.04.2023 21:17, Arınç ÜNAL wrote:
>> On 21.04.2023 20:28, Daniel Golle wrote:
>>> On Fri, Apr 21, 2023 at 05:36:34PM +0300, arinc9.unal@gmail.com wrote:
>>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>>
>>>> The idea of p5_interface and p6_interface pointers is to prevent
>>>> mt753x_mac_config() from running twice for MT7531, as it's already
>>>> run with
>>>> mt753x_cpu_port_enable() from mt7531_setup_common(), if the port is
>>>> used as
>>>> a CPU port.
>>>>
>>>> Change p5_interface and p6_interface to p5_configured and
>>>> p6_configured.
>>>> Make them boolean.
>>>>
>>>> Do not set them for any other reason.
>>>>
>>>> The priv->p5_intf_sel check is useless as in this code path, it will
>>>> always
>>>> be P5_INTF_SEL_GMAC5.
>>>>
>>>> There was also no need to set priv->p5_interface and
>>>> priv->p6_interface to
>>>> PHY_INTERFACE_MODE_NA on mt7530_setup() and mt7531_setup() as they
>>>> would
>>>> already be set to that when "priv" is allocated. The pointers were
>>>> of the
>>>> phy_interface_t enumeration type, and the first element of the enum is
>>>> PHY_INTERFACE_MODE_NA. There was nothing in between that would
>>>> change this
>>>> beforehand.
>>>>
>>>> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>
>>> NACK. This assumes that a user port is configured exactly once.
>>> However, interface mode may change because of mode-changing PHYs (e.g.
>>> often using Cisco SGMII for 10M/100M/1000M but using 2500Base-X for
>>> 2500M, ie. depending on actual link speed).
>>>
>>> Also when using SFP modules (which can be hotplugged) the interface
>>> mode may change after initially setting up the driver, e.g. when SFP
>>> driver is loaded or a module is plugged or replaced.
>>
>> I'm not sure I understand. pX_configured would be set to true only
>> when the port is used as a CPU port. mt753x_mac_config() should run
>> for user or DSA ports more than once, if needed.
>
> Looking at this again, once pX_interface is true, the check will prevent
> even user or DSA ports to be configured again. What about setting
> pX_interface to false after mt753x_mac_config() is run?
On a third thought, pX_interface will never be true for the port if it's
a user or DSA port so this should not be a problem at all.
Arınç
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 08/22] net: dsa: mt7530: change p{5,6}_interface to p{5,6}_configured
From: Arınç ÜNAL @ 2023-04-21 18:20 UTC (permalink / raw)
To: Daniel Golle
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <810aa47b-7007-7d53-9a23-c2d17d43d8a8@arinc9.com>
On 21.04.2023 21:17, Arınç ÜNAL wrote:
> On 21.04.2023 20:28, Daniel Golle wrote:
>> On Fri, Apr 21, 2023 at 05:36:34PM +0300, arinc9.unal@gmail.com wrote:
>>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>>
>>> The idea of p5_interface and p6_interface pointers is to prevent
>>> mt753x_mac_config() from running twice for MT7531, as it's already
>>> run with
>>> mt753x_cpu_port_enable() from mt7531_setup_common(), if the port is
>>> used as
>>> a CPU port.
>>>
>>> Change p5_interface and p6_interface to p5_configured and p6_configured.
>>> Make them boolean.
>>>
>>> Do not set them for any other reason.
>>>
>>> The priv->p5_intf_sel check is useless as in this code path, it will
>>> always
>>> be P5_INTF_SEL_GMAC5.
>>>
>>> There was also no need to set priv->p5_interface and
>>> priv->p6_interface to
>>> PHY_INTERFACE_MODE_NA on mt7530_setup() and mt7531_setup() as they would
>>> already be set to that when "priv" is allocated. The pointers were of
>>> the
>>> phy_interface_t enumeration type, and the first element of the enum is
>>> PHY_INTERFACE_MODE_NA. There was nothing in between that would change
>>> this
>>> beforehand.
>>>
>>> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> NACK. This assumes that a user port is configured exactly once.
>> However, interface mode may change because of mode-changing PHYs (e.g.
>> often using Cisco SGMII for 10M/100M/1000M but using 2500Base-X for
>> 2500M, ie. depending on actual link speed).
>>
>> Also when using SFP modules (which can be hotplugged) the interface
>> mode may change after initially setting up the driver, e.g. when SFP
>> driver is loaded or a module is plugged or replaced.
>
> I'm not sure I understand. pX_configured would be set to true only when
> the port is used as a CPU port. mt753x_mac_config() should run for user
> or DSA ports more than once, if needed.
Looking at this again, once pX_interface is true, the check will prevent
even user or DSA ports to be configured again. What about setting
pX_interface to false after mt753x_mac_config() is run?
Arınç
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH net-next 08/22] net: dsa: mt7530: change p{5,6}_interface to p{5,6}_configured
From: Arınç ÜNAL @ 2023-04-21 18:17 UTC (permalink / raw)
To: Daniel Golle
Cc: Sean Wang, Landen Chao, DENG Qingfang, Andrew Lunn,
Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, Russell King, Richard van Schagen,
Richard van Schagen, Frank Wunderlich, erkin.bozoglu, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek
In-Reply-To: <ZELH2RlYLPjJGx6Y@makrotopia.org>
On 21.04.2023 20:28, Daniel Golle wrote:
> On Fri, Apr 21, 2023 at 05:36:34PM +0300, arinc9.unal@gmail.com wrote:
>> From: Arınç ÜNAL <arinc.unal@arinc9.com>
>>
>> The idea of p5_interface and p6_interface pointers is to prevent
>> mt753x_mac_config() from running twice for MT7531, as it's already run with
>> mt753x_cpu_port_enable() from mt7531_setup_common(), if the port is used as
>> a CPU port.
>>
>> Change p5_interface and p6_interface to p5_configured and p6_configured.
>> Make them boolean.
>>
>> Do not set them for any other reason.
>>
>> The priv->p5_intf_sel check is useless as in this code path, it will always
>> be P5_INTF_SEL_GMAC5.
>>
>> There was also no need to set priv->p5_interface and priv->p6_interface to
>> PHY_INTERFACE_MODE_NA on mt7530_setup() and mt7531_setup() as they would
>> already be set to that when "priv" is allocated. The pointers were of the
>> phy_interface_t enumeration type, and the first element of the enum is
>> PHY_INTERFACE_MODE_NA. There was nothing in between that would change this
>> beforehand.
>>
>> Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
>
> NACK. This assumes that a user port is configured exactly once.
> However, interface mode may change because of mode-changing PHYs (e.g.
> often using Cisco SGMII for 10M/100M/1000M but using 2500Base-X for
> 2500M, ie. depending on actual link speed).
>
> Also when using SFP modules (which can be hotplugged) the interface
> mode may change after initially setting up the driver, e.g. when SFP
> driver is loaded or a module is plugged or replaced.
I'm not sure I understand. pX_configured would be set to true only when
the port is used as a CPU port. mt753x_mac_config() should run for user
or DSA ports more than once, if needed.
Arınç
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 6/9] dt-bindings: hwmon: Add HPE GXP PSU Support
From: Rob Herring @ 2023-04-21 18:13 UTC (permalink / raw)
To: nick.hawkins
Cc: verdun, linus.walleij, brgl, krzysztof.kozlowski+dt, jdelvare,
linux, linux, linux-gpio, devicetree, linux-kernel, linux-hwmon,
linux-arm-kernel
In-Reply-To: <20230418152824.110823-7-nick.hawkins@hpe.com>
On Tue, Apr 18, 2023 at 10:28:21AM -0500, nick.hawkins@hpe.com wrote:
> From: Nick Hawkins <nick.hawkins@hpe.com>
>
> Provide i2c register information and CPLD register information to the
> driver.
>
> Signed-off-by: Nick Hawkins <nick.hawkins@hpe.com>
> ---
> .../bindings/hwmon/hpe,gxp-psu.yaml | 45 +++++++++++++++++++
> 1 file changed, 45 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml b/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
> new file mode 100644
> index 000000000000..60ca0f6ace46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/hpe,gxp-psu.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/hpe,gxp-psu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HPE GXP psu controller
> +
> +maintainers:
> + - Nicholas Hawkins <nick.hawkins@hpe.com>
> +
> +properties:
> + compatible:
> + const: hpe,gxp-psu
> + interrupts:
> + maxItems: 1
> +
> + reg:
> + maxItems: 1
> +
> + hpe,sysreg:
Why is this optional?
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the global status registers shared between each psu
> + controller instance.
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + psu@48 {
> + compatible = "hpe,gxp-psu";
> + reg = <0x48>;
> + hpe,sysreg = <&sysreg_system_controller2>;
> + };
> + };
> --
> 2.17.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 3/5] dt-bindings: usb: dwc2: add support for Amlogic A1 SoC USB peripheral
From: Rob Herring @ 2023-04-21 18:01 UTC (permalink / raw)
To: Dmitry Rokosov
Cc: linux-usb, hanjie.lin, gregkh, linux-kernel, yue.wang, rockosov,
robh+dt, hminas, martin.blumenstingl, mturquette,
linux-arm-kernel, jbrunet, neil.armstrong, linux-phy, vkoul,
krzysztof.kozlowski+dt, linux-amlogic, khilman, kishon,
Thinh.Nguyen, kernel, devicetree
In-Reply-To: <20230418111612.19479-4-ddrokosov@sberdevices.ru>
On Tue, 18 Apr 2023 14:16:10 +0300, Dmitry Rokosov wrote:
> Provide the appropriate compatible string for the DWC2 IP that is found
> inside the Amlogic A1 SoC and used in peripheral mode.
>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
> ---
> Documentation/devicetree/bindings/usb/dwc2.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 4/9] KVM: selftests: Print read and write accesses of pages by vCPUs in dirty_log_perf_test
From: Vipin Sharma @ 2023-04-21 16:53 UTC (permalink / raw)
To: maz, oliver.upton, james.morse, suzuki.poulose, yuzenghui,
catalin.marinas, will, chenhuacai, aleksandar.qemu.devel,
tsbogend, anup, atishp, paul.walmsley, palmer, aou, seanjc,
pbonzini, dmatlack, ricarkol
Cc: linux-arm-kernel, kvmarm, linux-mips, kvm-riscv, linux-riscv,
linux-kselftest, kvm, linux-kernel, Vipin Sharma
In-Reply-To: <20230421165305.804301-1-vipinsh@google.com>
Fetch read and write accesses of pages from guest code and print count
across all vCPUs in dirty_log_perf_test.
This data provides progress made by vCPUs during dirty logging
operations. Since, vCPUs execute in lockstep with userspace dirty log
iterations, this metric is not very interesting. However, in future
commits when dirty_log_perf_test can execute vCPUs independently from
dirty log iterations then this metric can give good measure of vCPUs
performance during dirty logging.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
.../selftests/kvm/dirty_log_perf_test.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 338f03a4a550..0a08a3d21123 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -12,6 +12,7 @@
#include <stdlib.h>
#include <time.h>
#include <pthread.h>
+#include <stdatomic.h>
#include <linux/bitmap.h>
#include "kvm_util.h"
@@ -66,17 +67,22 @@ static u64 dirty_log_manual_caps;
static bool host_quit;
static int iteration;
static int vcpu_last_completed_iteration[KVM_MAX_VCPUS];
+static atomic_ullong total_reads;
+static atomic_ullong total_writes;
static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
{
struct kvm_vcpu *vcpu = vcpu_args->vcpu;
int vcpu_idx = vcpu_args->vcpu_idx;
uint64_t pages_count = 0;
+ uint64_t reads = 0;
+ uint64_t writes = 0;
struct kvm_run *run;
struct timespec start;
struct timespec ts_diff;
struct timespec total = (struct timespec){0};
struct timespec avg;
+ struct ucall uc = {};
int ret;
run = vcpu->run;
@@ -89,7 +95,7 @@ static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
ts_diff = timespec_elapsed(start);
TEST_ASSERT(ret == 0, "vcpu_run failed: %d\n", ret);
- TEST_ASSERT(get_ucall(vcpu, NULL) == UCALL_SYNC,
+ TEST_ASSERT(get_ucall(vcpu, &uc) == UCALL_SYNC,
"Invalid guest sync status: exit_reason=%s\n",
exit_reason_str(run->exit_reason));
@@ -101,6 +107,8 @@ static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
if (current_iteration) {
pages_count += vcpu_args->pages;
total = timespec_add(total, ts_diff);
+ reads += uc.args[2];
+ writes += uc.args[3];
pr_debug("vCPU %d iteration %d dirty memory time: %ld.%.9lds\n",
vcpu_idx, current_iteration, ts_diff.tv_sec,
ts_diff.tv_nsec);
@@ -123,6 +131,8 @@ static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
pr_debug("\nvCPU %d dirtied 0x%lx pages over %d iterations in %ld.%.9lds. (Avg %ld.%.9lds/iteration)\n",
vcpu_idx, pages_count, vcpu_last_completed_iteration[vcpu_idx],
total.tv_sec, total.tv_nsec, avg.tv_sec, avg.tv_nsec);
+ atomic_fetch_add(&total_reads, reads);
+ atomic_fetch_add(&total_writes, writes);
}
struct test_params {
@@ -176,6 +186,8 @@ static void run_test(enum vm_guest_mode mode, void *arg)
dirty_log_manual_caps);
arch_setup_vm(vm, nr_vcpus);
+ atomic_store(&total_reads, 0);
+ atomic_store(&total_writes, 0);
/* Start the iterations */
iteration = 0;
@@ -295,6 +307,10 @@ static void run_test(enum vm_guest_mode mode, void *arg)
clear_dirty_log_total.tv_nsec, avg.tv_sec, avg.tv_nsec);
}
+ pr_info("Total pages touched: %llu (Reads: %llu, Writes: %llu)\n",
+ atomic_load(&total_reads) + atomic_load(&total_writes),
+ atomic_load(&total_reads), atomic_load(&total_writes));
+
memstress_free_bitmaps(bitmaps, p->slots);
arch_cleanup_vm(vm);
memstress_destroy_vm(vm);
--
2.40.0.634.g4ca3ef3211-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 5/9] KVM: selftests: Allow independent execution of vCPUs in dirty_log_perf_test
From: Vipin Sharma @ 2023-04-21 16:53 UTC (permalink / raw)
To: maz, oliver.upton, james.morse, suzuki.poulose, yuzenghui,
catalin.marinas, will, chenhuacai, aleksandar.qemu.devel,
tsbogend, anup, atishp, paul.walmsley, palmer, aou, seanjc,
pbonzini, dmatlack, ricarkol
Cc: linux-arm-kernel, kvmarm, linux-mips, kvm-riscv, linux-riscv,
linux-kselftest, kvm, linux-kernel, Vipin Sharma
In-Reply-To: <20230421165305.804301-1-vipinsh@google.com>
Allow vCPUs to execute independent of dirty log iterations after
initialization is complete. Hide this feature behind the new option
"-j".
This change makes dirty_log_perf_test execute like real world workflows
where guest vCPUs keep on executing while VMM collects dirty logs. Total
pages touched during execution of test will give good estimate of how
vCPUs are performing while dirty logging is enabled.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
.../selftests/kvm/dirty_log_perf_test.c | 60 ++++++++++++-------
1 file changed, 40 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 0a08a3d21123..ffdad535fdaa 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -69,6 +69,7 @@ static int iteration;
static int vcpu_last_completed_iteration[KVM_MAX_VCPUS];
static atomic_ullong total_reads;
static atomic_ullong total_writes;
+static bool lockstep_iterations;
static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
{
@@ -83,12 +84,16 @@ static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
struct timespec total = (struct timespec){0};
struct timespec avg;
struct ucall uc = {};
+ int current_iteration = -1;
int ret;
run = vcpu->run;
while (!READ_ONCE(host_quit)) {
- int current_iteration = READ_ONCE(iteration);
+ if (lockstep_iterations)
+ current_iteration = READ_ONCE(iteration);
+ else
+ current_iteration++;
clock_gettime(CLOCK_MONOTONIC, &start);
ret = _vcpu_run(vcpu);
@@ -118,13 +123,19 @@ static void vcpu_worker(struct memstress_vcpu_args *vcpu_args)
ts_diff.tv_nsec);
}
- /*
- * Keep running the guest while dirty logging is being disabled
- * (iteration is negative) so that vCPUs are accessing memory
- * for the entire duration of zapping collapsible SPTEs.
- */
- while (current_iteration == READ_ONCE(iteration) &&
- READ_ONCE(iteration) >= 0 && !READ_ONCE(host_quit)) {}
+ if (lockstep_iterations) {
+ /*
+ * Keep running the guest while dirty logging is being disabled
+ * (iteration is negative) so that vCPUs are accessing memory
+ * for the entire duration of zapping collapsible SPTEs.
+ */
+ while (current_iteration == READ_ONCE(iteration) &&
+ READ_ONCE(iteration) >= 0 && !READ_ONCE(host_quit))
+ ;
+ } else {
+ while (!READ_ONCE(iteration))
+ ;
+ }
}
avg = timespec_div(total, vcpu_last_completed_iteration[vcpu_idx]);
@@ -238,17 +249,19 @@ static void run_test(enum vm_guest_mode mode, void *arg)
clock_gettime(CLOCK_MONOTONIC, &start);
iteration++;
- pr_debug("Starting iteration %d\n", iteration);
- for (i = 0; i < nr_vcpus; i++) {
- while (READ_ONCE(vcpu_last_completed_iteration[i])
- != iteration)
- ;
- }
+ if (lockstep_iterations) {
+ pr_debug("Starting iteration %d\n", iteration);
+ for (i = 0; i < nr_vcpus; i++) {
+ while (READ_ONCE(vcpu_last_completed_iteration[i])
+ != iteration)
+ ;
+ }
- ts_diff = timespec_elapsed(start);
- vcpu_dirty_total = timespec_add(vcpu_dirty_total, ts_diff);
- pr_info("Iteration %d dirty memory time: %ld.%.9lds\n",
- iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
+ ts_diff = timespec_elapsed(start);
+ vcpu_dirty_total = timespec_add(vcpu_dirty_total, ts_diff);
+ pr_info("Iteration %d dirty memory time: %ld.%.9lds\n",
+ iteration, ts_diff.tv_sec, ts_diff.tv_nsec);
+ }
clock_gettime(CLOCK_MONOTONIC, &start);
memstress_get_dirty_log(vm, bitmaps, p->slots);
@@ -365,6 +378,10 @@ static void help(char *name)
" To leave the application task unpinned, drop the final entry:\n\n"
" ./dirty_log_perf_test -v 3 -c 22,23,24\n\n"
" (default: no pinning)\n");
+ printf(" -j: Execute vCPUs independent of dirty log iterations\n"
+ " Independent vCPUs execution will allow them to continuously\n"
+ " dirty memory while main thread is collecting and clearing\n"
+ " dirty logs in the main thread's iterations.\n");
printf(" -k: Specify the chunk size in which dirty memory gets cleared\n"
" in memslots in each iteration. If the size is bigger than\n"
" the memslot size then whole memslot is cleared in one call.\n"
@@ -399,10 +416,10 @@ int main(int argc, char *argv[])
kvm_check_cap(KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2);
dirty_log_manual_caps &= (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE |
KVM_DIRTY_LOG_INITIALLY_SET);
-
+ lockstep_iterations = true;
guest_modes_append_default();
- while ((opt = getopt(argc, argv, "ab:c:eghi:k:l:m:nop:r:s:v:x:w:")) != -1) {
+ while ((opt = getopt(argc, argv, "ab:c:eghi:jk:l:m:nop:r:s:v:x:w:")) != -1) {
switch (opt) {
case 'a':
p.random_access = true;
@@ -426,6 +443,9 @@ int main(int argc, char *argv[])
case 'i':
p.iterations = atoi_positive("Number of iterations", optarg);
break;
+ case 'j':
+ lockstep_iterations = false;
+ break;
case 'k':
p.clear_chunk_size = parse_size(optarg);
break;
--
2.40.0.634.g4ca3ef3211-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/9] KVM: selftests: Allow dirty_log_perf_test to clear dirty memory in chunks
From: Vipin Sharma @ 2023-04-21 16:52 UTC (permalink / raw)
To: maz, oliver.upton, james.morse, suzuki.poulose, yuzenghui,
catalin.marinas, will, chenhuacai, aleksandar.qemu.devel,
tsbogend, anup, atishp, paul.walmsley, palmer, aou, seanjc,
pbonzini, dmatlack, ricarkol
Cc: linux-arm-kernel, kvmarm, linux-mips, kvm-riscv, linux-riscv,
linux-kselftest, kvm, linux-kernel, Vipin Sharma
In-Reply-To: <20230421165305.804301-1-vipinsh@google.com>
In dirty_log_perf_test, provide option 'k' to specify the size of the
chunks and clear dirty memory in chunks in each iteration. If this
option is not provided then fallback to old way of clearing whole
memslot in one call per iteration.
In production environment whole memslot is rarely cleared in a single
call, instead clearing operation is split across multiple calls to
reduce time between clearing and sending memory to a remote host. This
change mimics the production usecases and allow to get metrics based on
that.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
.../selftests/kvm/dirty_log_perf_test.c | 19 ++++++++++++---
.../testing/selftests/kvm/include/memstress.h | 12 ++++++++--
tools/testing/selftests/kvm/lib/memstress.c | 24 ++++++++++++++-----
3 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/kvm/dirty_log_perf_test.c b/tools/testing/selftests/kvm/dirty_log_perf_test.c
index 416719e20518..0852a7ba42e1 100644
--- a/tools/testing/selftests/kvm/dirty_log_perf_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_perf_test.c
@@ -134,6 +134,7 @@ struct test_params {
uint32_t write_percent;
uint32_t random_seed;
bool random_access;
+ uint64_t clear_chunk_size;
};
static void run_test(enum vm_guest_mode mode, void *arg)
@@ -144,6 +145,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
uint64_t guest_num_pages;
uint64_t host_num_pages;
uint64_t pages_per_slot;
+ uint64_t pages_per_clear;
struct timespec start;
struct timespec ts_diff;
struct timespec get_dirty_log_total = (struct timespec){0};
@@ -164,6 +166,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
host_num_pages = vm_num_host_pages(mode, guest_num_pages);
pages_per_slot = host_num_pages / p->slots;
+ pages_per_clear = p->clear_chunk_size / getpagesize();
bitmaps = memstress_alloc_bitmaps(p->slots, pages_per_slot);
@@ -244,8 +247,9 @@ static void run_test(enum vm_guest_mode mode, void *arg)
if (dirty_log_manual_caps) {
clock_gettime(CLOCK_MONOTONIC, &start);
- memstress_clear_dirty_log(vm, bitmaps, p->slots,
- pages_per_slot);
+ memstress_clear_dirty_log_in_chunks(vm, bitmaps, p->slots,
+ pages_per_slot,
+ pages_per_clear);
ts_diff = timespec_elapsed(start);
clear_dirty_log_total = timespec_add(clear_dirty_log_total,
ts_diff);
@@ -343,6 +347,11 @@ static void help(char *name)
" To leave the application task unpinned, drop the final entry:\n\n"
" ./dirty_log_perf_test -v 3 -c 22,23,24\n\n"
" (default: no pinning)\n");
+ printf(" -k: Specify the chunk size in which dirty memory gets cleared\n"
+ " in memslots in each iteration. If the size is bigger than\n"
+ " the memslot size then whole memslot is cleared in one call.\n"
+ " Size must be aligned to the host page size. e.g. 10M or 3G\n"
+ " (default: UINT64_MAX, clears whole memslot in one call)\n");
puts("");
exit(0);
}
@@ -358,6 +367,7 @@ int main(int argc, char *argv[])
.slots = 1,
.random_seed = 1,
.write_percent = 100,
+ .clear_chunk_size = UINT64_MAX,
};
int opt;
@@ -368,7 +378,7 @@ int main(int argc, char *argv[])
guest_modes_append_default();
- while ((opt = getopt(argc, argv, "ab:c:eghi:m:nop:r:s:v:x:w:")) != -1) {
+ while ((opt = getopt(argc, argv, "ab:c:eghi:k:m:nop:r:s:v:x:w:")) != -1) {
switch (opt) {
case 'a':
p.random_access = true;
@@ -392,6 +402,9 @@ int main(int argc, char *argv[])
case 'i':
p.iterations = atoi_positive("Number of iterations", optarg);
break;
+ case 'k':
+ p.clear_chunk_size = parse_size(optarg);
+ break;
case 'm':
guest_modes_cmdline(optarg);
break;
diff --git a/tools/testing/selftests/kvm/include/memstress.h b/tools/testing/selftests/kvm/include/memstress.h
index ce4e603050ea..2acc93f76fc3 100644
--- a/tools/testing/selftests/kvm/include/memstress.h
+++ b/tools/testing/selftests/kvm/include/memstress.h
@@ -75,8 +75,16 @@ void memstress_setup_nested(struct kvm_vm *vm, int nr_vcpus, struct kvm_vcpu *vc
void memstress_enable_dirty_logging(struct kvm_vm *vm, int slots);
void memstress_disable_dirty_logging(struct kvm_vm *vm, int slots);
void memstress_get_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[], int slots);
-void memstress_clear_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[],
- int slots, uint64_t pages_per_slot);
+void memstress_clear_dirty_log_in_chunks(struct kvm_vm *vm,
+ unsigned long *bitmaps[], int slots,
+ uint64_t pages_per_slot,
+ uint64_t pages_per_clear);
+static inline void memstress_clear_dirty_log(struct kvm_vm *vm,
+ unsigned long *bitmaps[], int slots,
+ uint64_t pages_per_slot) {
+ memstress_clear_dirty_log_in_chunks(vm, bitmaps, slots, pages_per_slot,
+ pages_per_slot);
+}
unsigned long **memstress_alloc_bitmaps(int slots, uint64_t pages_per_slot);
void memstress_free_bitmaps(unsigned long *bitmaps[], int slots);
diff --git a/tools/testing/selftests/kvm/lib/memstress.c b/tools/testing/selftests/kvm/lib/memstress.c
index 3632956c6bcf..e0c701ab4e9a 100644
--- a/tools/testing/selftests/kvm/lib/memstress.c
+++ b/tools/testing/selftests/kvm/lib/memstress.c
@@ -355,16 +355,28 @@ void memstress_get_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[], int sl
}
}
-void memstress_clear_dirty_log(struct kvm_vm *vm, unsigned long *bitmaps[],
- int slots, uint64_t pages_per_slot)
+void memstress_clear_dirty_log_in_chunks(struct kvm_vm *vm,
+ unsigned long *bitmaps[], int slots,
+ uint64_t pages_per_slot,
+ uint64_t pages_per_clear)
{
- int i;
+ int i, slot;
+ uint64_t from, clear_pages_count;
for (i = 0; i < slots; i++) {
- int slot = MEMSTRESS_MEM_SLOT_INDEX + i;
-
- kvm_vm_clear_dirty_log(vm, slot, bitmaps[i], 0, pages_per_slot);
+ slot = MEMSTRESS_MEM_SLOT_INDEX + i;
+ from = 0;
+ clear_pages_count = pages_per_clear;
+
+ while (from < pages_per_slot) {
+ if (from + clear_pages_count > pages_per_slot)
+ clear_pages_count = pages_per_slot - from;
+ kvm_vm_clear_dirty_log(vm, slot, bitmaps[i], from,
+ clear_pages_count);
+ from += clear_pages_count;
+ }
}
+
}
unsigned long **memstress_alloc_bitmaps(int slots, uint64_t pages_per_slot)
--
2.40.0.634.g4ca3ef3211-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 3/9] KVM: selftests: Pass count of read and write accesses from guest to host
From: Vipin Sharma @ 2023-04-21 16:52 UTC (permalink / raw)
To: maz, oliver.upton, james.morse, suzuki.poulose, yuzenghui,
catalin.marinas, will, chenhuacai, aleksandar.qemu.devel,
tsbogend, anup, atishp, paul.walmsley, palmer, aou, seanjc,
pbonzini, dmatlack, ricarkol
Cc: linux-arm-kernel, kvmarm, linux-mips, kvm-riscv, linux-riscv,
linux-kselftest, kvm, linux-kernel, Vipin Sharma
In-Reply-To: <20230421165305.804301-1-vipinsh@google.com>
Pass number of read and write accesses done in the memstress' guest code
to userspace.
These counts will be one way to measure vCPU performances during
memstress and dirty logging related tests. For example, in
dirty_log_perf_test this can be used to measure impact of dirty and
clear log APIs on vCPUs performances.
In current dirty_log_perf_test, each vCPU executes in lockstep to the
current iteration in userspace, therefore, these access counts will not
provide much useful information except for observing individual vCPUs
read vs write accesses.
However, in future commits, dirty_log_perf_test behavior will be changed
to allow vCPUs to execute independent of userspace iterations. This will
mimic real world workload where guest keeps on executing while VMM is
collecting and clearing dirty logs separately. With read and write
accesses known for each vCPU, impact of get and clear dirty log APIs can
be quantified. Note that these access counts will not be 100% reliable
in knowing vCPUs performances since vCPUs scheduling can impact
the progress.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
tools/testing/selftests/kvm/lib/memstress.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/memstress.c b/tools/testing/selftests/kvm/lib/memstress.c
index 483ecbc53a5b..9c2e360e610f 100644
--- a/tools/testing/selftests/kvm/lib/memstress.c
+++ b/tools/testing/selftests/kvm/lib/memstress.c
@@ -50,6 +50,8 @@ void memstress_guest_code(uint32_t vcpu_idx)
struct memstress_args *args = &memstress_args;
struct memstress_vcpu_args *vcpu_args = &args->vcpu_args[vcpu_idx];
struct guest_random_state rand_state;
+ uint64_t write_access;
+ uint64_t read_access;
uint64_t gva;
uint64_t pages;
uint64_t addr;
@@ -65,6 +67,8 @@ void memstress_guest_code(uint32_t vcpu_idx)
GUEST_ASSERT(vcpu_args->vcpu_idx == vcpu_idx);
while (true) {
+ write_access = 0;
+ read_access = 0;
for (i = 0; i < pages; i++) {
if (args->random_access)
page = guest_random_u32(&rand_state) % pages;
@@ -73,13 +77,16 @@ void memstress_guest_code(uint32_t vcpu_idx)
addr = gva + (page * args->guest_page_size);
- if (guest_random_u32(&rand_state) % 100 < args->write_percent)
+ if (guest_random_u32(&rand_state) % 100 < args->write_percent) {
*(uint64_t *)addr = 0x0123456789ABCDEF;
- else
+ write_access++;
+ } else {
READ_ONCE(*(uint64_t *)addr);
+ read_access++;
+ }
}
- GUEST_SYNC(1);
+ GUEST_SYNC_ARGS(1, read_access, write_access, 0, 0);
}
}
--
2.40.0.634.g4ca3ef3211-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH V7 2/4] dt-bindings: clock: document Amlogic S4 SoC peripherals clock controller
From: Rob Herring @ 2023-04-21 17:50 UTC (permalink / raw)
To: Yu Tu
Cc: linux-arm-kernel, kelvin.zhang, devicetree, Krzysztof Kozlowski,
Stephen Boyd, Martin Blumenstingl, linux-amlogic,
Michael Turquette, qi.duan, linux-kernel, linux-clk, Rob Herring,
Neil Armstrong, Jerome Brunet, Kevin Hilman
In-Reply-To: <20230417065005.24967-3-yu.tu@amlogic.com>
On Mon, 17 Apr 2023 14:50:03 +0800, Yu Tu wrote:
> Add the S4 peripherals clock controller dt-bindings in the s4 SoC
> family.
>
> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
> ---
> .../clock/amlogic,s4-peripherals-clkc.yaml | 97 +++++++++++++
> .../clock/amlogic,s4-peripherals-clkc.h | 131 ++++++++++++++++++
> 2 files changed, 228 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml
> create mode 100644 include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h
>
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V7 1/4] dt-bindings: clock: document Amlogic S4 SoC PLL clock controller
From: Rob Herring @ 2023-04-21 17:49 UTC (permalink / raw)
To: Yu Tu
Cc: linux-amlogic, devicetree, linux-clk, Neil Armstrong,
linux-arm-kernel, Stephen Boyd, qi.duan, Martin Blumenstingl,
Krzysztof Kozlowski, Rob Herring, Kevin Hilman, kelvin.zhang,
linux-kernel, Michael Turquette, Jerome Brunet
In-Reply-To: <20230417065005.24967-2-yu.tu@amlogic.com>
On Mon, 17 Apr 2023 14:50:02 +0800, Yu Tu wrote:
> Add the S4 PLL clock controller dt-bindings in the s4 SoC family.
>
> Signed-off-by: Yu Tu <yu.tu@amlogic.com>
> ---
> .../bindings/clock/amlogic,s4-pll-clkc.yaml | 50 +++++++++++++++++++
> MAINTAINERS | 1 +
> .../dt-bindings/clock/amlogic,s4-pll-clkc.h | 30 +++++++++++
> 3 files changed, 81 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml
> create mode 100644 include/dt-bindings/clock/amlogic,s4-pll-clkc.h
>
Reviewed-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 8/9] KMV: arm64: Allow stage2_apply_range_sched() to pass page table walker flags
From: Vipin Sharma @ 2023-04-21 16:53 UTC (permalink / raw)
To: maz, oliver.upton, james.morse, suzuki.poulose, yuzenghui,
catalin.marinas, will, chenhuacai, aleksandar.qemu.devel,
tsbogend, anup, atishp, paul.walmsley, palmer, aou, seanjc,
pbonzini, dmatlack, ricarkol
Cc: linux-arm-kernel, kvmarm, linux-mips, kvm-riscv, linux-riscv,
linux-kselftest, kvm, linux-kernel, Vipin Sharma
In-Reply-To: <20230421165305.804301-1-vipinsh@google.com>
Allow stage2_apply_range_sched() to pass enum kvm_pgtable_walk_flags{}
to stage 2 walkers. Pass 0 as the flag to make this change no-op
This capability will be used in future commits to enable clear-dirty-log
operation under MMU read lock.
Current users of stage2_apply_range_*() API run under assumption of
holding MMU write lock. Stage2 page table walkers then run under the
same assumption. In future commits when clear-dirty-log operation under
MMU read lock is added then there needs to be a way to pass this shared
intent to page table walkers.
No functional changes intended.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
arch/arm64/include/asm/kvm_pgtable.h | 12 +++++++++---
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 4 ++--
arch/arm64/kvm/hyp/pgtable.c | 16 ++++++++++------
arch/arm64/kvm/mmu.c | 26 ++++++++++++++++----------
4 files changed, 37 insertions(+), 21 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 4cd62506c198..79a452d78e08 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -508,6 +508,7 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
* @pgt: Page-table structure initialised by kvm_pgtable_stage2_init*().
* @addr: Intermediate physical address from which to remove the mapping.
* @size: Size of the mapping.
+ * @flags: Page-table walker flags.
*
* The offset of @addr within a page is ignored and @size is rounded-up to
* the next page boundary.
@@ -520,7 +521,8 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
*
* Return: 0 on success, negative error code on failure.
*/
-int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size);
+int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size,
+ enum kvm_pgtable_walk_flags flags);
/**
* kvm_pgtable_stage2_wrprotect() - Write-protect guest stage-2 address range
@@ -528,6 +530,7 @@ int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size);
* @pgt: Page-table structure initialised by kvm_pgtable_stage2_init*().
* @addr: Intermediate physical address from which to write-protect,
* @size: Size of the range.
+ * @flags: Page-table walker flags.
*
* The offset of @addr within a page is ignored and @size is rounded-up to
* the next page boundary.
@@ -538,7 +541,8 @@ int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size);
*
* Return: 0 on success, negative error code on failure.
*/
-int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size);
+int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size,
+ enum kvm_pgtable_walk_flags flags);
/**
* kvm_pgtable_stage2_mkyoung() - Set the access flag in a page-table entry.
@@ -610,13 +614,15 @@ bool kvm_pgtable_stage2_is_young(struct kvm_pgtable *pgt, u64 addr);
* @pgt: Page-table structure initialised by kvm_pgtable_stage2_init*().
* @addr: Intermediate physical address from which to flush.
* @size: Size of the range.
+ * @flags: Page-table walker flags.
*
* The offset of @addr within a page is ignored and @size is rounded-up to
* the next page boundary.
*
* Return: 0 on success, negative error code on failure.
*/
-int kvm_pgtable_stage2_flush(struct kvm_pgtable *pgt, u64 addr, u64 size);
+int kvm_pgtable_stage2_flush(struct kvm_pgtable *pgt, u64 addr, u64 size,
+ enum kvm_pgtable_walk_flags flags);
/**
* kvm_pgtable_walk() - Walk a page-table.
diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index 552653fa18be..bac3c2c31cbe 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -326,11 +326,11 @@ static int host_stage2_unmap_dev_all(void)
/* Unmap all non-memory regions to recycle the pages */
for (i = 0; i < hyp_memblock_nr; i++, addr = reg->base + reg->size) {
reg = &hyp_memory[i];
- ret = kvm_pgtable_stage2_unmap(pgt, addr, reg->base - addr);
+ ret = kvm_pgtable_stage2_unmap(pgt, addr, reg->base - addr, 0);
if (ret)
return ret;
}
- return kvm_pgtable_stage2_unmap(pgt, addr, BIT(pgt->ia_bits) - addr);
+ return kvm_pgtable_stage2_unmap(pgt, addr, BIT(pgt->ia_bits) - addr, 0);
}
struct kvm_mem_range {
diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
index 3d61bd3e591d..3a585e1fba11 100644
--- a/arch/arm64/kvm/hyp/pgtable.c
+++ b/arch/arm64/kvm/hyp/pgtable.c
@@ -1024,12 +1024,14 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx,
return 0;
}
-int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size)
+int kvm_pgtable_stage2_unmap(struct kvm_pgtable *pgt, u64 addr, u64 size,
+ enum kvm_pgtable_walk_flags flags)
{
struct kvm_pgtable_walker walker = {
.cb = stage2_unmap_walker,
.arg = pgt,
- .flags = KVM_PGTABLE_WALK_LEAF | KVM_PGTABLE_WALK_TABLE_POST,
+ .flags = flags | KVM_PGTABLE_WALK_LEAF |
+ KVM_PGTABLE_WALK_TABLE_POST,
};
return kvm_pgtable_walk(pgt, addr, size, &walker);
@@ -1108,11 +1110,12 @@ static int stage2_update_leaf_attrs(struct kvm_pgtable *pgt, u64 addr,
return 0;
}
-int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size)
+int kvm_pgtable_stage2_wrprotect(struct kvm_pgtable *pgt, u64 addr, u64 size,
+ enum kvm_pgtable_walk_flags flags)
{
return stage2_update_leaf_attrs(pgt, addr, size, 0,
KVM_PTE_LEAF_ATTR_LO_S2_S2AP_W,
- NULL, NULL, 0);
+ NULL, NULL, flags);
}
kvm_pte_t kvm_pgtable_stage2_mkyoung(struct kvm_pgtable *pgt, u64 addr)
@@ -1193,11 +1196,12 @@ static int stage2_flush_walker(const struct kvm_pgtable_visit_ctx *ctx,
return 0;
}
-int kvm_pgtable_stage2_flush(struct kvm_pgtable *pgt, u64 addr, u64 size)
+int kvm_pgtable_stage2_flush(struct kvm_pgtable *pgt, u64 addr, u64 size,
+ enum kvm_pgtable_walk_flags flags)
{
struct kvm_pgtable_walker walker = {
.cb = stage2_flush_walker,
- .flags = KVM_PGTABLE_WALK_LEAF,
+ .flags = flags | KVM_PGTABLE_WALK_LEAF,
.arg = pgt,
};
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index dc1c9059604e..e0189cdda43d 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -48,7 +48,9 @@ static phys_addr_t stage2_range_addr_end(phys_addr_t addr, phys_addr_t end)
*/
static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
phys_addr_t end,
- int (*fn)(struct kvm_pgtable *, u64, u64),
+ enum kvm_pgtable_walk_flags flags,
+ int (*fn)(struct kvm_pgtable *, u64, u64,
+ enum kvm_pgtable_walk_flags),
bool resched)
{
struct kvm *kvm = kvm_s2_mmu_to_kvm(mmu);
@@ -61,7 +63,7 @@ static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
return -EINVAL;
next = stage2_range_addr_end(addr, end);
- ret = fn(pgt, addr, next - addr);
+ ret = fn(pgt, addr, next - addr, flags);
if (ret)
break;
@@ -72,8 +74,8 @@ static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
return ret;
}
-#define stage2_apply_range_resched(mmu, addr, end, fn) \
- stage2_apply_range(mmu, addr, end, fn, true)
+#define stage2_apply_range_resched(mmu, addr, end, flags, fn) \
+ stage2_apply_range(mmu, addr, end, flags, fn, true)
static bool memslot_is_logging(struct kvm_memory_slot *memslot)
{
@@ -236,7 +238,7 @@ static void __unmap_stage2_range(struct kvm_s2_mmu *mmu, phys_addr_t start, u64
lockdep_assert_held_write(&kvm->mmu_lock);
WARN_ON(size & ~PAGE_MASK);
- WARN_ON(stage2_apply_range(mmu, start, end, kvm_pgtable_stage2_unmap,
+ WARN_ON(stage2_apply_range(mmu, start, end, 0, kvm_pgtable_stage2_unmap,
may_block));
}
@@ -251,7 +253,8 @@ static void stage2_flush_memslot(struct kvm *kvm,
phys_addr_t addr = memslot->base_gfn << PAGE_SHIFT;
phys_addr_t end = addr + PAGE_SIZE * memslot->npages;
- stage2_apply_range_resched(&kvm->arch.mmu, addr, end, kvm_pgtable_stage2_flush);
+ stage2_apply_range_resched(&kvm->arch.mmu, addr, end, 0,
+ kvm_pgtable_stage2_flush);
}
/**
@@ -932,10 +935,13 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
* @mmu: The KVM stage-2 MMU pointer
* @addr: Start address of range
* @end: End address of range
+ * @flags: Page-table walker flags.
*/
-static void stage2_wp_range(struct kvm_s2_mmu *mmu, phys_addr_t addr, phys_addr_t end)
+static void stage2_wp_range(struct kvm_s2_mmu *mmu, phys_addr_t addr, phys_addr_t end,
+ enum kvm_pgtable_walk_flags flags)
{
- stage2_apply_range_resched(mmu, addr, end, kvm_pgtable_stage2_wrprotect);
+ stage2_apply_range_resched(mmu, addr, end, flags,
+ kvm_pgtable_stage2_wrprotect);
}
/**
@@ -964,7 +970,7 @@ static void kvm_mmu_wp_memory_region(struct kvm *kvm, int slot)
end = (memslot->base_gfn + memslot->npages) << PAGE_SHIFT;
write_lock(&kvm->mmu_lock);
- stage2_wp_range(&kvm->arch.mmu, start, end);
+ stage2_wp_range(&kvm->arch.mmu, start, end, 0);
write_unlock(&kvm->mmu_lock);
kvm_flush_remote_tlbs(kvm);
}
@@ -988,7 +994,7 @@ static void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
phys_addr_t start = (base_gfn + __ffs(mask)) << PAGE_SHIFT;
phys_addr_t end = (base_gfn + __fls(mask) + 1) << PAGE_SHIFT;
- stage2_wp_range(&kvm->arch.mmu, start, end);
+ stage2_wp_range(&kvm->arch.mmu, start, end, 0);
}
/*
--
2.40.0.634.g4ca3ef3211-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 9/9] KVM: arm64: Run clear-dirty-log under MMU read lock
From: Vipin Sharma @ 2023-04-21 16:53 UTC (permalink / raw)
To: maz, oliver.upton, james.morse, suzuki.poulose, yuzenghui,
catalin.marinas, will, chenhuacai, aleksandar.qemu.devel,
tsbogend, anup, atishp, paul.walmsley, palmer, aou, seanjc,
pbonzini, dmatlack, ricarkol
Cc: linux-arm-kernel, kvmarm, linux-mips, kvm-riscv, linux-riscv,
linux-kselftest, kvm, linux-kernel, Vipin Sharma
In-Reply-To: <20230421165305.804301-1-vipinsh@google.com>
Take MMU read lock for write protecting PTEs and use shared page table
walker for clearing dirty logs.
Clearing dirty logs are currently performed under MMU write locks. This
means vCPUs write protection fault, which also take MMU read lock, will
be blocked during this operation. This causes guest degradation and
especially noticeable on VMs with lot of vCPUs.
Taking MMU read lock will allow vCPUs to execute parallelly and reduces
the impact on vCPUs performance.
Tested improvement on a ARM Ampere Altra host (64 CPUs, 256 GB
memory and single NUMA node) via dirty_log_perf_test for 48 vCPU, 96
GB memory, 8GB clear chunk size, 1 second wait between Clear-Dirty-Log
calls and configuration:
Test command:
./dirty_log_perf_test -s anonymous_hugetlb_2mb -b 2G -v 48 -l 1 -k 8G -j -m 2
Before:
Total pages touched: 50331648 (Reads: 0, Writes: 50331648)
After:
Total pages touched: 125304832 (Reads: 0, Writes: 125304832)
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
arch/arm64/kvm/mmu.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index e0189cdda43d..3f2117d93998 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -67,8 +67,12 @@ static int stage2_apply_range(struct kvm_s2_mmu *mmu, phys_addr_t addr,
if (ret)
break;
- if (resched && next != end)
- cond_resched_rwlock_write(&kvm->mmu_lock);
+ if (resched && next != end) {
+ if (flags & KVM_PGTABLE_WALK_SHARED)
+ cond_resched_rwlock_read(&kvm->mmu_lock);
+ else
+ cond_resched_rwlock_write(&kvm->mmu_lock);
+ }
} while (addr = next, addr != end);
return ret;
@@ -994,7 +998,7 @@ static void kvm_mmu_write_protect_pt_masked(struct kvm *kvm,
phys_addr_t start = (base_gfn + __ffs(mask)) << PAGE_SHIFT;
phys_addr_t end = (base_gfn + __fls(mask) + 1) << PAGE_SHIFT;
- stage2_wp_range(&kvm->arch.mmu, start, end, 0);
+ stage2_wp_range(&kvm->arch.mmu, start, end, KVM_PGTABLE_WALK_SHARED);
}
/*
@@ -1008,9 +1012,9 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
struct kvm_memory_slot *slot,
gfn_t gfn_offset, unsigned long mask)
{
- write_lock(&kvm->mmu_lock);
+ read_lock(&kvm->mmu_lock);
kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
- write_unlock(&kvm->mmu_lock);
+ read_unlock(&kvm->mmu_lock);
}
static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
--
2.40.0.634.g4ca3ef3211-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 7/9] KVM: mmu: Move mmu lock/unlock to arch code for clear dirty log
From: Vipin Sharma @ 2023-04-21 16:53 UTC (permalink / raw)
To: maz, oliver.upton, james.morse, suzuki.poulose, yuzenghui,
catalin.marinas, will, chenhuacai, aleksandar.qemu.devel,
tsbogend, anup, atishp, paul.walmsley, palmer, aou, seanjc,
pbonzini, dmatlack, ricarkol
Cc: linux-arm-kernel, kvmarm, linux-mips, kvm-riscv, linux-riscv,
linux-kselftest, kvm, linux-kernel, Vipin Sharma
In-Reply-To: <20230421165305.804301-1-vipinsh@google.com>
Move mmu_lock lock and unlock calls from common code in
kvm_clear_dirty_log_protect() to arch specific code in
kvm_arch_mmu_enable_log_dirty_pt_masked(). None of the other code inside
the for loop of kvm_arch_mmu_enable_log_dirty_pt_masked() needs mmu_lock
exclusivity apart from the arch specific API call.
Future commits will change clear dirty log operations under mmu read
lock instead of write lock for ARM and, potentially, x86 architectures.
No functional changes intended.
Signed-off-by: Vipin Sharma <vipinsh@google.com>
---
arch/arm64/kvm/mmu.c | 2 ++
arch/mips/kvm/mmu.c | 2 ++
arch/riscv/kvm/mmu.c | 2 ++
arch/x86/kvm/mmu/mmu.c | 3 +++
virt/kvm/dirty_ring.c | 2 --
virt/kvm/kvm_main.c | 4 ----
6 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 7113587222ff..dc1c9059604e 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1002,7 +1002,9 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
struct kvm_memory_slot *slot,
gfn_t gfn_offset, unsigned long mask)
{
+ write_lock(&kvm->mmu_lock);
kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
+ write_unlock(&kvm->mmu_lock);
}
static void kvm_send_hwpoison_signal(unsigned long address, short lsb)
diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c
index e8c08988ed37..b8d4723d197e 100644
--- a/arch/mips/kvm/mmu.c
+++ b/arch/mips/kvm/mmu.c
@@ -415,11 +415,13 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
struct kvm_memory_slot *slot,
gfn_t gfn_offset, unsigned long mask)
{
+ spin_lock(&kvm->mmu_lock);
gfn_t base_gfn = slot->base_gfn + gfn_offset;
gfn_t start = base_gfn + __ffs(mask);
gfn_t end = base_gfn + __fls(mask);
kvm_mips_mkclean_gpa_pt(kvm, start, end);
+ spin_unlock(&kvm->mmu_lock);
}
/*
diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
index 78211aed36fa..425fa11dcf9c 100644
--- a/arch/riscv/kvm/mmu.c
+++ b/arch/riscv/kvm/mmu.c
@@ -395,11 +395,13 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
gfn_t gfn_offset,
unsigned long mask)
{
+ spin_lock(&kvm->mmu_lock);
phys_addr_t base_gfn = slot->base_gfn + gfn_offset;
phys_addr_t start = (base_gfn + __ffs(mask)) << PAGE_SHIFT;
phys_addr_t end = (base_gfn + __fls(mask) + 1) << PAGE_SHIFT;
gstage_wp_range(kvm, start, end);
+ spin_unlock(&kvm->mmu_lock);
}
void kvm_arch_sync_dirty_log(struct kvm *kvm, struct kvm_memory_slot *memslot)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 144c5a01cd77..f1dc549b01cb 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1367,6 +1367,7 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
struct kvm_memory_slot *slot,
gfn_t gfn_offset, unsigned long mask)
{
+ write_lock(&kvm->mmu_lock);
/*
* Huge pages are NOT write protected when we start dirty logging in
* initially-all-set mode; must write protect them here so that they
@@ -1397,6 +1398,8 @@ void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm,
kvm_mmu_clear_dirty_pt_masked(kvm, slot, gfn_offset, mask);
else
kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask);
+
+ write_unlock(&kvm->mmu_lock);
}
int kvm_cpu_dirty_log_size(void)
diff --git a/virt/kvm/dirty_ring.c b/virt/kvm/dirty_ring.c
index c1cd7dfe4a90..d894c58d2152 100644
--- a/virt/kvm/dirty_ring.c
+++ b/virt/kvm/dirty_ring.c
@@ -66,9 +66,7 @@ static void kvm_reset_dirty_gfn(struct kvm *kvm, u32 slot, u64 offset, u64 mask)
if (!memslot || (offset + __fls(mask)) >= memslot->npages)
return;
- KVM_MMU_LOCK(kvm);
kvm_arch_mmu_enable_log_dirty_pt_masked(kvm, memslot, offset, mask);
- KVM_MMU_UNLOCK(kvm);
}
int kvm_dirty_ring_alloc(struct kvm_dirty_ring *ring, int index, u32 size)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index f40b72eb0e7b..378c40e958b6 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2157,7 +2157,6 @@ static int kvm_get_dirty_log_protect(struct kvm *kvm, struct kvm_dirty_log *log)
dirty_bitmap_buffer = kvm_second_dirty_bitmap(memslot);
memset(dirty_bitmap_buffer, 0, n);
- KVM_MMU_LOCK(kvm);
for (i = 0; i < n / sizeof(long); i++) {
unsigned long mask;
gfn_t offset;
@@ -2173,7 +2172,6 @@ static int kvm_get_dirty_log_protect(struct kvm *kvm, struct kvm_dirty_log *log)
kvm_arch_mmu_enable_log_dirty_pt_masked(kvm, memslot,
offset, mask);
}
- KVM_MMU_UNLOCK(kvm);
}
if (flush)
@@ -2268,7 +2266,6 @@ static int kvm_clear_dirty_log_protect(struct kvm *kvm,
if (copy_from_user(dirty_bitmap_buffer, log->dirty_bitmap, n))
return -EFAULT;
- KVM_MMU_LOCK(kvm);
for (offset = log->first_page, i = offset / BITS_PER_LONG,
n = DIV_ROUND_UP(log->num_pages, BITS_PER_LONG); n--;
i++, offset += BITS_PER_LONG) {
@@ -2291,7 +2288,6 @@ static int kvm_clear_dirty_log_protect(struct kvm *kvm,
offset, mask);
}
}
- KVM_MMU_UNLOCK(kvm);
if (flush)
kvm_arch_flush_remote_tlbs_memslot(kvm, memslot);
--
2.40.0.634.g4ca3ef3211-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox