* [PATCH] usb: dwc3: Check that the request is valid in dwc3_gadget_giveback()
From: Marek Vasut @ 2020-02-14 18:33 UTC (permalink / raw)
To: u-boot
In-Reply-To: <20200214122328.24987-1-vigneshr@ti.com>
On 2/14/20 1:23 PM, Vignesh Raghavendra wrote:
> From: Jean-Jacques Hiblot <jjhiblot@ti.com>
>
> This fixes potential issues reported by klokworks:
> Pointer 'req' returned from call to function 'next_request' at line 531 and
> 538 may be NULL and will be dereferenced in dwc3_gadget_giveback()
Shouldn't you rather handle the issue in dwc3_remove_requests() ?
Also, please explain what conditions trigger this issue, i.e. when req
becomes NULL.
^ permalink raw reply
* Re: [PATCH net-next v2] net: phy: dp83867: Add speed optimization feature
From: Grygorii Strashko @ 2020-02-14 18:32 UTC (permalink / raw)
To: Dan Murphy, Florian Fainelli, Heiner Kallweit, andrew
Cc: linux, davem, netdev, linux-kernel
In-Reply-To: <c7a7bd71-3a1c-1cf3-5faa-204b10ea8b78@ti.com>
On 06/02/2020 00:01, Dan Murphy wrote:
> Florian
>
> On 2/5/20 4:00 PM, Florian Fainelli wrote:
>> On 2/5/20 1:51 PM, Dan Murphy wrote:
>>> Heiner
>>>
>>> On 2/5/20 3:16 PM, Heiner Kallweit wrote:
>>>> On 04.02.2020 19:13, Dan Murphy wrote:
>>>>> Set the speed optimization bit on the DP83867 PHY.
>>>>> This feature can also be strapped on the 64 pin PHY devices
>>>>> but the 48 pin devices do not have the strap pin available to enable
>>>>> this feature in the hardware. PHY team suggests to have this bit set.
>>>>>
>>>>> With this bit set the PHY will auto negotiate and report the link
>>>>> parameters in the PHYSTS register. This register provides a single
>>>>> location within the register set for quick access to commonly accessed
>>>>> information.
>>>>>
>>>>> In this case when auto negotiation is on the PHY core reads the bits
>>>>> that have been configured or if auto negotiation is off the PHY core
>>>>> reads the BMCR register and sets the phydev parameters accordingly.
>>>>>
>>>>> This Giga bit PHY can throttle the speed to 100Mbps or 10Mbps to
>>>>> accomodate a
>>>>> 4-wire cable. If this should occur the PHYSTS register contains the
>>>>> current negotiated speed and duplex mode.
>>>>>
>>>>> In overriding the genphy_read_status the dp83867_read_status will do a
>>>>> genphy_read_status to setup the LP and pause bits. And then the PHYSTS
>>>>> register is read and the phydev speed and duplex mode settings are
>>>>> updated.
>>>>>
>>>>> Signed-off-by: Dan Murphy <dmurphy@ti.com>
>>>>> ---
>>>>> v2 - Updated read status to call genphy_read_status first, added
>>>>> link_change
>>>>> callback to notify of speed change and use phy_set_bits -
>>>>> https://lore.kernel.org/patchwork/patch/1188348/
>>>>>
>>>> As stated in the first review, it would be appreciated if you implement
>>>> also the downshift tunable. This could be a separate patch in this
>>>> series.
>>>> Most of the implementation would be boilerplate code.
>>> I just don't have a requirement from our customer to make it adjustable
>>> so I did not want to add something extra.
>>>
>>> I can add in for v3.
>>>
>>>> And I have to admit that I'm not too happy with the term "speed
>>>> optimization".
>>>> This sounds like the PHY has some magic to establish a 1.2Gbps link.
>>>> Even though the vendor may call it this way in the datasheet, the
>>>> standard
>>>> term is "downshift". I'm fine with using "speed optimization" in
>>>> constants
>>>> to be in line with the datasheet. Just a comment in the code would be
>>>> helpful
>>>> that speed optimization is the vendor's term for downshift.
>>> Ack. The data sheet actually says "Speed optimization, also known as
>>> link downshift"
>>>
>>> So I probably will just rename everything down shift.
>>>
>>>>> drivers/net/phy/dp83867.c | 55 +++++++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 55 insertions(+)
>>>>>
>>>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
>>>>> index 967f57ed0b65..6f86ca1ebb51 100644
>>>>> --- a/drivers/net/phy/dp83867.c
>>>>> +++ b/drivers/net/phy/dp83867.c
>>>>> @@ -21,6 +21,7 @@
>>>>> #define DP83867_DEVADDR 0x1f
>>>>> #define MII_DP83867_PHYCTRL 0x10
>>>>> +#define MII_DP83867_PHYSTS 0x11
>>>>> #define MII_DP83867_MICR 0x12
>>>>> #define MII_DP83867_ISR 0x13
>>>>> #define DP83867_CFG2 0x14
>>>>> @@ -118,6 +119,15 @@
>>>>> #define DP83867_IO_MUX_CFG_CLK_O_SEL_MASK (0x1f << 8)
>>>>> #define DP83867_IO_MUX_CFG_CLK_O_SEL_SHIFT 8
>>>>> +/* PHY STS bits */
>>>>> +#define DP83867_PHYSTS_1000 BIT(15)
>>>>> +#define DP83867_PHYSTS_100 BIT(14)
>>>>> +#define DP83867_PHYSTS_DUPLEX BIT(13)
>>>>> +#define DP83867_PHYSTS_LINK BIT(10)
>>>>> +
>>>>> +/* CFG2 bits */
>>>>> +#define DP83867_SPEED_OPTIMIZED_EN (BIT(8) | BIT(9))
>>>>> +
>>>>> /* CFG3 bits */
>>>>> #define DP83867_CFG3_INT_OE BIT(7)
>>>>> #define DP83867_CFG3_ROBUST_AUTO_MDIX BIT(9)
>>>>> @@ -287,6 +297,43 @@ static int dp83867_config_intr(struct phy_device
>>>>> *phydev)
>>>>> return phy_write(phydev, MII_DP83867_MICR, micr_status);
>>>>> }
>>>>> +static void dp83867_link_change_notify(struct phy_device *phydev)
>>>>> +{
>>>>> + if (phydev->state != PHY_RUNNING)
>>>>> + return;
>>>>> +
>>>>> + if (phydev->speed == SPEED_100 || phydev->speed == SPEED_10)
>>>>> + phydev_warn(phydev, "Downshift detected connection is
>>>>> %iMbps\n",
>>>>> + phydev->speed);
>>>> The link partner may simply not advertise 1Gbps. How do you know that
>>>> a link speed of e.g. 100Mbps is caused by a downshift?
>>>> Some PHY's I've seen with this feature have a flag somewhere indicating
>>>> that downshift occurred. How about the PHY here?
>>> I don't see a register that gives us that status
>>>
>>> I will ask the hardware team if there is one.
>>>
>>> This is a 1Gbps PHY by default so if a slower connection is established
>>> due to faulty cabling or LP advertisement then this would be a down
>>> shift IMO.
>> With your current link_change_notify function it would not be possible
>> to know whether the PHY was connected to a link partner that advertised
>> only 10/100 and so 100 ended up being the link speed, or the link
>> partner was capable of 10/100/1000 and downshift reduced the link speed.
>>
>> If you cannot tell the difference from a register, it might be better to
>> simply omit that function then.
>
> Yeah I thought it was a bit redundant and wonky to see in the log that the link established to xG/Mbps and then see another message saying the downshift occurred.
I think it's good idea to have this message as just wrong cable might be used.
But this notifier make no sense in it current form - it will produce noise in case of forced 100m/10M.
FYI. PHY sequence to update link:
phy_state_machine()
|-phy_check_link_status()
|-phy_link_down/up()
|- .phy_link_change()->phy_link_change()
|-adjust_link() ----> netdev callback
|-phydev->drv->link_change_notify(phydev);
So, log output has to be done or in .read_status() or
some info has to be saved in .read_status() and then re-used in
.link_change_notify().
--
Best regards,
grygorii
^ permalink raw reply
* Re: [V2 2/3] firmware: arm_sdei: Removed multiple white lines.
From: James Morse @ 2020-02-14 18:32 UTC (permalink / raw)
To: luanshi; +Cc: linux-arm-kernel, linux-kernel
In-Reply-To: <1579145331-78633-2-git-send-email-zhangliguang@linux.alibaba.com>
Hi Luanshi,
On 16/01/2020 03:28, luanshi wrote:
> Remove one unnecessary white line.
>
> Signed-off-by: Liguang Zhang <zhangliguang@linux.alibaba.com>
> ---
> drivers/firmware/arm_sdei.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
> index 37e9bf0..f81c09e 100644
> --- a/drivers/firmware/arm_sdei.c
> +++ b/drivers/firmware/arm_sdei.c
> @@ -599,7 +599,6 @@ static int _sdei_event_register(struct sdei_event *event)
> event->registered,
> SDEI_EVENT_REGISTER_RM_ANY, 0);
>
> -
> err = sdei_do_cross_call(_local_event_register, event);
> if (err) {
> spin_lock(&event->sdei_event_lock);
I'm afraid these whitespace-only patches aren't worth sending. If its not caught at
review, it gets to annoy the reader until someone can do a drive-by fix when they are
changing adjacent code.
I've merged this with the first patch in the eventual series.
Thanks,
James
^ permalink raw reply
* Re: [V2 1/3] firmware: arm_sdei: fix possible deadlock
From: James Morse @ 2020-02-14 18:32 UTC (permalink / raw)
To: luanshi; +Cc: linux-arm-kernel, linux-kernel
In-Reply-To: <1579145331-78633-1-git-send-email-zhangliguang@linux.alibaba.com>
Hi Luanshi,
On 16/01/2020 03:28, luanshi wrote:
> We call sdei_reregister_event() with sdei_list_lock held but
> _sdei_event_register() and sdei_event_destroy() also acquires
> sdei_list_lock thus creating A-A deadlock.
>
> Fixes: da351827240e ("firmware: arm_sdei: Add support for CPU and system
> power states")
>
(Nit: stray whitespace in the fixes tag, the backport tools may choke on this)
(Please include 'PATCH' in the [] section of the subject when posting, its part of the
'canonical patch format', and my scripts for pulling a series of the list depend on it!)
> ---
Thanks for picking up my suggestion, ... it was what I think should have been done in the
first place to avoid this bug.
Looking at your patch, we'd need to take the per-event lock around the reads of reregister
and reenable in sdei_cpuhp_up() too, and sdei_reregister_shared(), ... and this quickly
becomes much noisier than a patch for stable should be. (Sorry, I should have tried it
before suggesting it!)
I've picked up your first version, but instead of duplicating the contents of the
function, I've added '_llocked' wrappers to account for that lock already being held. This
isn't great as we have _locked too, but lockdep should keep us honest.
Because I started with your patch, git has kept you as author.
This ended up as patch 2, because it was also necessary to move those reregister updates
into their callers to fix hibernate.
I'll posted what I have next week, sorry for the hiatus.
Thanks,
James
^ permalink raw reply
* Re: Debugging errors with Dell XPS 9560 TPM
From: Alex Guzman @ 2020-02-14 18:32 UTC (permalink / raw)
To: linux-integrity
In-Reply-To: <CAJ7-PMaLw_H8Fc1tyoT95f5EWpS3nvJt1Wx9=xpuiSotJ2h9VA@mail.gmail.com>
Looks like someone had a look on the bug tracker
(https://bugzilla.kernel.org/show_bug.cgi?id=206275#c6)
and they figure it's definitely a regression in the kernel and should
be reverted or rectified. They advised me to come ping here once more.
- Alex
On Sat, Feb 1, 2020 at 4:19 PM Alex Guzman <alex@guzman.io> wrote:
>
> Hey there! I reported a bug on the bug tracker a bit ago but haven't
> seen any movement, so I figured I'd drop in here. My XPS 9560 has been
> having issues with its TPM, and all commands will fail with error 1
> when operating on the TPM device. I managed to bisect it back to
> commit 4d6ebc4c4950595414722dfadd0b361f5a05d37e (tpm: fix invalid
> locking in NONBLOCKING mode) though it began failing with error 14
> (bad address) at that point.
>
> I reported the bug at
> https://bugzilla.kernel.org/show_bug.cgi?id=206275 and attached some
> dmesg logs from boot there. Does anyone have any suggestions for
> additional debugging or such to figure out what's happening here?
>
> - Alex
^ permalink raw reply
* [PATCH v2] ARM: qcom_defconfig: Enable QRTR
From: Luca Weiss @ 2020-02-14 18:31 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arm-msm, Linus Walleij, Russell King, Krzysztof Kozlowski,
Bjorn Andersson, Luca Weiss, Jordan Crouse,
~postmarketos/upstreaming, Matti Lehtimäki, Georgi Djakov,
linux-kernel, Brian Masney
This option is useful on msm8974, so enable it.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
Changes from v1:
- set options as =m instead of =y
arch/arm/configs/qcom_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/qcom_defconfig b/arch/arm/configs/qcom_defconfig
index ad3417a63cdf..4702feef2cc9 100644
--- a/arch/arm/configs/qcom_defconfig
+++ b/arch/arm/configs/qcom_defconfig
@@ -44,6 +44,8 @@ CONFIG_IP_ROUTE_VERBOSE=y
CONFIG_IP_PNP=y
CONFIG_IP_PNP_DHCP=y
# CONFIG_IPV6 is not set
+CONFIG_QRTR=m
+CONFIG_QRTR_SMD=m
CONFIG_CFG80211=m
CONFIG_MAC80211=m
CONFIG_RFKILL=y
--
2.25.0
_______________________________________________
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 AUTOSEL 5.4 191/459] ARM: OMAP2+: Add workaround for DRA7 DSP MStandby errata i879
From: Suman Anna @ 2020-02-14 18:32 UTC (permalink / raw)
To: Sasha Levin, linux-kernel, stable
Cc: Tony Lindgren, linux-omap, linux-arm-kernel
In-Reply-To: <20200214160149.11681-191-sashal@kernel.org>
Hi Sasha,
On 2/14/20 9:57 AM, Sasha Levin wrote:
> From: Suman Anna <s-anna@ti.com>
>
> [ Upstream commit 2f14101a1d760db72393910d481fbf7768c44530 ]
>
> Errata Title:
> i879: DSP MStandby requires CD_EMU in SW_WKUP
>
> Description:
> The DSP requires the internal emulation clock to be actively toggling
> in order to successfully enter a low power mode via execution of the
> IDLE instruction and PRCM MStandby/Idle handshake. This assumes that
> other prerequisites and software sequence are followed.
>
> Workaround:
> The emulation clock to the DSP is free-running anytime CCS is connected
> via JTAG debugger to the DSP subsystem or when the CD_EMU clock domain
> is set in SW_WKUP mode. The CD_EMU domain can be set in SW_WKUP mode
> via the CM_EMU_CLKSTCTRL [1:0]CLKTRCTRL field.
>
> Implementation:
> This patch implements this workaround by denying the HW_AUTO mode
> for the EMU clockdomain during the power-up of any DSP processor
> and re-enabling the HW_AUTO mode during the shutdown of the last
> DSP processor (actually done during the enabling and disabling of
> the respective DSP MDMA MMUs). Reference counting has to be used to
> manage the independent sequencing between the multiple DSP processors.
>
> This switching is done at runtime rather than a static clockdomain
> flags value to meet the target power domain state for the EMU power
> domain during suspend.
>
> Note that the DSP MStandby behavior is not consistent across all
> boards prior to this fix. Please see commit 45f871eec6c0 ("ARM:
> OMAP2+: Extend DRA7 IPU1 MMU pdata quirks to DSP MDMA MMUs") for
> details.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
You can drop this from the 5.4-stable queue. Mainline doesn't yet boot
the processors, so this is not needed for stable queue.
regards
Suman
> ---
> arch/arm/mach-omap2/omap-iommu.c | 43 +++++++++++++++++++++++++++++---
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
> index f1a6ece8108e4..78247e6f4a720 100644
> --- a/arch/arm/mach-omap2/omap-iommu.c
> +++ b/arch/arm/mach-omap2/omap-iommu.c
> @@ -11,14 +11,43 @@
>
> #include "omap_hwmod.h"
> #include "omap_device.h"
> +#include "clockdomain.h"
> #include "powerdomain.h"
>
> +static void omap_iommu_dra7_emu_swsup_config(struct platform_device *pdev,
> + bool enable)
> +{
> + static struct clockdomain *emu_clkdm;
> + static DEFINE_SPINLOCK(emu_lock);
> + static atomic_t count;
> + struct device_node *np = pdev->dev.of_node;
> +
> + if (!of_device_is_compatible(np, "ti,dra7-dsp-iommu"))
> + return;
> +
> + if (!emu_clkdm) {
> + emu_clkdm = clkdm_lookup("emu_clkdm");
> + if (WARN_ON_ONCE(!emu_clkdm))
> + return;
> + }
> +
> + spin_lock(&emu_lock);
> +
> + if (enable && (atomic_inc_return(&count) == 1))
> + clkdm_deny_idle(emu_clkdm);
> + else if (!enable && (atomic_dec_return(&count) == 0))
> + clkdm_allow_idle(emu_clkdm);
> +
> + spin_unlock(&emu_lock);
> +}
> +
> int omap_iommu_set_pwrdm_constraint(struct platform_device *pdev, bool request,
> u8 *pwrst)
> {
> struct powerdomain *pwrdm;
> struct omap_device *od;
> u8 next_pwrst;
> + int ret = 0;
>
> od = to_omap_device(pdev);
> if (!od)
> @@ -31,13 +60,21 @@ int omap_iommu_set_pwrdm_constraint(struct platform_device *pdev, bool request,
> if (!pwrdm)
> return -EINVAL;
>
> - if (request)
> + if (request) {
> *pwrst = pwrdm_read_next_pwrst(pwrdm);
> + omap_iommu_dra7_emu_swsup_config(pdev, true);
> + }
>
> if (*pwrst > PWRDM_POWER_RET)
> - return 0;
> + goto out;
>
> next_pwrst = request ? PWRDM_POWER_ON : *pwrst;
>
> - return pwrdm_set_next_pwrst(pwrdm, next_pwrst);
> + ret = pwrdm_set_next_pwrst(pwrdm, next_pwrst);
> +
> +out:
> + if (!request)
> + omap_iommu_dra7_emu_swsup_config(pdev, false);
> +
> + 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
* Re: [PATCH AUTOSEL 5.4 191/459] ARM: OMAP2+: Add workaround for DRA7 DSP MStandby errata i879
From: Suman Anna @ 2020-02-14 18:32 UTC (permalink / raw)
To: Sasha Levin, linux-kernel, stable
Cc: Tony Lindgren, linux-arm-kernel, linux-omap
In-Reply-To: <20200214160149.11681-191-sashal@kernel.org>
Hi Sasha,
On 2/14/20 9:57 AM, Sasha Levin wrote:
> From: Suman Anna <s-anna@ti.com>
>
> [ Upstream commit 2f14101a1d760db72393910d481fbf7768c44530 ]
>
> Errata Title:
> i879: DSP MStandby requires CD_EMU in SW_WKUP
>
> Description:
> The DSP requires the internal emulation clock to be actively toggling
> in order to successfully enter a low power mode via execution of the
> IDLE instruction and PRCM MStandby/Idle handshake. This assumes that
> other prerequisites and software sequence are followed.
>
> Workaround:
> The emulation clock to the DSP is free-running anytime CCS is connected
> via JTAG debugger to the DSP subsystem or when the CD_EMU clock domain
> is set in SW_WKUP mode. The CD_EMU domain can be set in SW_WKUP mode
> via the CM_EMU_CLKSTCTRL [1:0]CLKTRCTRL field.
>
> Implementation:
> This patch implements this workaround by denying the HW_AUTO mode
> for the EMU clockdomain during the power-up of any DSP processor
> and re-enabling the HW_AUTO mode during the shutdown of the last
> DSP processor (actually done during the enabling and disabling of
> the respective DSP MDMA MMUs). Reference counting has to be used to
> manage the independent sequencing between the multiple DSP processors.
>
> This switching is done at runtime rather than a static clockdomain
> flags value to meet the target power domain state for the EMU power
> domain during suspend.
>
> Note that the DSP MStandby behavior is not consistent across all
> boards prior to this fix. Please see commit 45f871eec6c0 ("ARM:
> OMAP2+: Extend DRA7 IPU1 MMU pdata quirks to DSP MDMA MMUs") for
> details.
>
> Signed-off-by: Suman Anna <s-anna@ti.com>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
You can drop this from the 5.4-stable queue. Mainline doesn't yet boot
the processors, so this is not needed for stable queue.
regards
Suman
> ---
> arch/arm/mach-omap2/omap-iommu.c | 43 +++++++++++++++++++++++++++++---
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap-iommu.c b/arch/arm/mach-omap2/omap-iommu.c
> index f1a6ece8108e4..78247e6f4a720 100644
> --- a/arch/arm/mach-omap2/omap-iommu.c
> +++ b/arch/arm/mach-omap2/omap-iommu.c
> @@ -11,14 +11,43 @@
>
> #include "omap_hwmod.h"
> #include "omap_device.h"
> +#include "clockdomain.h"
> #include "powerdomain.h"
>
> +static void omap_iommu_dra7_emu_swsup_config(struct platform_device *pdev,
> + bool enable)
> +{
> + static struct clockdomain *emu_clkdm;
> + static DEFINE_SPINLOCK(emu_lock);
> + static atomic_t count;
> + struct device_node *np = pdev->dev.of_node;
> +
> + if (!of_device_is_compatible(np, "ti,dra7-dsp-iommu"))
> + return;
> +
> + if (!emu_clkdm) {
> + emu_clkdm = clkdm_lookup("emu_clkdm");
> + if (WARN_ON_ONCE(!emu_clkdm))
> + return;
> + }
> +
> + spin_lock(&emu_lock);
> +
> + if (enable && (atomic_inc_return(&count) == 1))
> + clkdm_deny_idle(emu_clkdm);
> + else if (!enable && (atomic_dec_return(&count) == 0))
> + clkdm_allow_idle(emu_clkdm);
> +
> + spin_unlock(&emu_lock);
> +}
> +
> int omap_iommu_set_pwrdm_constraint(struct platform_device *pdev, bool request,
> u8 *pwrst)
> {
> struct powerdomain *pwrdm;
> struct omap_device *od;
> u8 next_pwrst;
> + int ret = 0;
>
> od = to_omap_device(pdev);
> if (!od)
> @@ -31,13 +60,21 @@ int omap_iommu_set_pwrdm_constraint(struct platform_device *pdev, bool request,
> if (!pwrdm)
> return -EINVAL;
>
> - if (request)
> + if (request) {
> *pwrst = pwrdm_read_next_pwrst(pwrdm);
> + omap_iommu_dra7_emu_swsup_config(pdev, true);
> + }
>
> if (*pwrst > PWRDM_POWER_RET)
> - return 0;
> + goto out;
>
> next_pwrst = request ? PWRDM_POWER_ON : *pwrst;
>
> - return pwrdm_set_next_pwrst(pwrdm, next_pwrst);
> + ret = pwrdm_set_next_pwrst(pwrdm, next_pwrst);
> +
> +out:
> + if (!request)
> + omap_iommu_dra7_emu_swsup_config(pdev, false);
> +
> + return ret;
> }
>
^ permalink raw reply
* Re: [PATCH] credential.c: fix credential reading with regards to CR/LF
From: Jeff King @ 2020-02-14 18:32 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget
Cc: git, Johannes Schindelin, Nikita Leonov
In-Reply-To: <pull.710.git.git.1581688196706.gitgitgadget@gmail.com>
On Fri, Feb 14, 2020 at 01:49:56PM +0000, Johannes Schindelin via GitGitGadget wrote:
> From: Nikita Leonov <nykyta.leonov@gmail.com>
>
> This fix makes using Git credentials more friendly to Windows users. In
> previous version it was unable to finish input correctly without
> configuration changes (tested in PowerShell, CMD, Cygwin).
>
> We know credential filling should be finished by empty input, but the
> current implementation does not take into account CR/LF ending, and
> hence instead of the empty string we get '\r', which is interpreted as
> an incorrect string.
>
> So this commit changes default reading function to a more Windows
> compatible reading function.
This does make it impossible to have a CR at the end of a data value. I
think that should be OK (we already disallow LF with no mechanism for
quoting, because who the hell puts a LF in their password?).
But we should perhaps update the section in git-credential(1) that
describes the rules. I had trouble coming up with a wording that wasn't
totally awkward, though:
diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index 6f0c7ca80f..09e4b58321 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -112,7 +112,9 @@ specified by a key-value pair, separated by an `=` (equals) sign,
followed by a newline. The key may contain any bytes except `=`,
newline, or NUL. The value may contain any bytes except newline or NUL.
In both cases, all bytes are treated as-is (i.e., there is no quoting,
-and one cannot transmit a value with newline or NUL in it). The list of
+and one cannot transmit a value with newline or NUL in it). Note that
+Git will treat a carriage return before the final newline as part of
+line ending, and not part of the data. The list of
attributes is terminated by a blank line or end-of-file.
Git understands the following attributes:
This is talking about the git-credential tool itself, but the actual
helper protocol documentation links to this. (As an aside, I notice that
the protocol documentation recently got moved into credential.h along
with the C API bits. Yuck. That probably should be in
gitcredentials(7)).
-Peff
^ permalink raw reply related
* [Bug 1863025] Re: Use-after-free after flush in TCG accelerator
From: Yifan @ 2020-02-14 18:18 UTC (permalink / raw)
To: qemu-devel
In-Reply-To: <158154486735.14935.3370403781300872079.malonedeb@soybean.canonical.com>
Apologies, the patch got messed up.
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index c01f59c743..7a9e8c94bd 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -238,8 +238,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
uint32_t flags;
uint32_t cflags = 1;
uint32_t cf_mask = cflags & CF_HASH_MASK;
+ unsigned flush_count;
if (sigsetjmp(cpu->jmp_env, 0) == 0) {
+retry:
+ flush_count = tb_flush_count();
tb = tb_lookup__cpu_state(cpu, &pc, &cs_base, &flags, cf_mask);
if (tb == NULL) {
mmap_lock();
@@ -248,6 +251,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
}
start_exclusive();
+ /* do_tb_flush() might run and make tb invalid */
+ if (flush_count != tb_flush_count()) {
+ end_exclusive();
+ goto retry;
+ }
/* Since we got here, we know that parallel_cpus must be true. */
parallel_cpus = false;
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 9f48da9472..2fb7da9b51 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2674,3 +2674,8 @@ void tcg_flush_softmmu_tlb(CPUState *cs)
tlb_flush(cs);
#endif
}
+
+unsigned tb_flush_count(void)
+{
+ return atomic_read(&tb_ctx.tb_flush_count);
+}
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index d85e610e85..aa3c2d219a 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -579,6 +579,9 @@ void tlb_set_dirty(CPUState *cpu, target_ulong vaddr);
/* exec.c */
void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr);
+/* translate-all.c */
+unsigned tb_flush_count(void);
+
MemoryRegionSection *
address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
hwaddr *xlat, hwaddr *plen,
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1863025
Title:
Use-after-free after flush in TCG accelerator
Status in QEMU:
Confirmed
Bug description:
I believe I found a UAF in TCG that can lead to a guest VM escape. The
security list informed me "This can not be treated as a security
issue." and to post it here. I am looking at the 4.2.0 source code.
The issue requires a race and I will try to describe it in terms of
three concurrent threads.
Thread A:
A1. qemu_tcg_cpu_thread_fn runs work loop
A2. qemu_wait_io_event => qemu_wait_io_event_common => process_queued_cpu_work
A3. start_exclusive critical section entered
A4. do_tb_flush is called, TB memory freed/re-allocated
A5. end_exclusive exits critical section
Thread B:
B1. qemu_tcg_cpu_thread_fn runs work loop
B2. tcg_cpu_exec => cpu_exec => tb_find => tb_gen_code
B3. tcg_tb_alloc obtains a new TB
Thread C:
C1. qemu_tcg_cpu_thread_fn runs work loop
C2. cpu_exec_step_atomic executes
C3. TB obtained with tb_lookup__cpu_state or tb_gen_code
C4. start_exclusive critical section entered
C5. cpu_tb_exec executes the TB code
C6. end_exclusive exits critical section
Consider the following sequence of events:
B2 => B3 => C3 (same TB as B2) => A3 => A4 (TB freed) => A5 => B2 =>
B3 (re-allocates TB from B2) => C4 => C5 (freed/reused TB now executing) => C6
In short, because thread C uses the TB in the critical section, there
is no guarantee that the pointer has not been "freed" (rather the
memory is marked as re-usable) and therefore a use-after-free occurs.
Since the TCG generated code can be in the same memory as the TB data
structure, it is possible for an attacker to overwrite the UAF pointer
with code generated from TCG. This can overwrite key pointer values
and could lead to code execution on the host outside of the TCG
sandbox.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1863025/+subscriptions
^ permalink raw reply related
* Re: [PATCH v4 2/6] clocksource: Ingenic: Add high resolution timer support for SMP.
From: Paul Cercueil @ 2020-02-14 18:31 UTC (permalink / raw)
To: 周琰杰 (Zhou Yanjie)
Cc: linux-mips, linux-clk, linux-kernel, devicetree, mturquette,
sboyd, robh+dt, mark.rutland, ralf, paulburton, jiaxun.yang,
chenhc, allison, tglx, daniel.lezcano, geert+renesas, krzk,
keescook, ebiederm, miquel.raynal, paul, hns, sernia.zhou,
zhenwenjin, mips-creator-ci20-dev, 1326991897
In-Reply-To: <1581703360-112557-4-git-send-email-zhouyanjie@wanyeetech.com>
Hi Zhou,
Le sam., févr. 15, 2020 at 02:02, 周琰杰 (Zhou Yanjie)
<zhouyanjie@wanyeetech.com> a écrit :
> Enable clock event handling on per CPU core basis.
> Make sure that interrupts raised on the first core execute
> event handlers on the correct CPU core.
>
> Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
> Tested-by: Paul Boddie <paul@boddie.org.uk>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>
> Notes:
> v1->v2:
> 1.Adjust function naming to make it more reasonable.
> 2.Replace function smp_call_function_single() with
> smp_call_function_single_async() in order to resolve
> the warning below:
>
> [ 0.350942] smp: Brought up 1 node, 2 CPUs
> [ 0.365497] ------------[ cut here ]------------
> [ 0.365522] WARNING: CPU: 0 PID: 1 at kernel/smp.c:300
> smp_call_function_single+0x110/0x200
> [ 0.365533] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 5.5.0-rc1+ #5
> [ 0.365537] Stack : 00000000 59c73bcd 00000037 80074e80
> 80000000 80670000 805a0000 80620590
> [ 0.365557] 8065ce38 8fc0dc8c 806d0000 00000000
> 80670000 00000001 8fc0dc20 59c73bcd
> [ 0.365574] 00000000 00000000 806f0000 80670000
> 00000000 806dab00 00000000 2d302e35
> [ 0.365591] 203a6d6d 806e0000 806e0000 70617773
> 80670000 00000000 00000000 00000009
> [ 0.365610] 00000000 8fc94e20 8fc0de30 80690000
> 00000018 803592dc 00000000 806d0000
> [ 0.365627] ...
> [ 0.365634] Call Trace:
> [ 0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
> [ 0.365663] [<804aed20>] dump_stack+0x98/0xc8
> [ 0.365673] [<8003044c>] __warn+0xc4/0xe8
> [ 0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
> [ 0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
> [ 0.365703] ---[ end trace 5785856ca39c79d5 ]---
> [ 0.365557] 8065ce38 8fc0dc8c 806d0000 00000000
> 80670000 00000001 8fc0dc20 59c73bcd
> [ 0.365574] 00000000 00000000 806f0000 80670000
> 00000000 806dab00 00000000 2d302e35
> [ 0.365591] 203a6d6d 806e0000 806e0000 70617773
> 80670000 00000000 00000000 00000009
> [ 0.365610] 00000000 8fc94e20 8fc0de30 80690000
> 00000018 803592dc 00000000 806d0000
> [ 0.365627] ...
> [ 0.365634] Call Trace:
> [ 0.365647] [<8001b9a0>] show_stack+0x6c/0x12c
> [ 0.365663] [<804aed20>] dump_stack+0x98/0xc8
> [ 0.365673] [<8003044c>] __warn+0xc4/0xe8
> [ 0.365682] [<800304f4>] warn_slowpath_fmt+0x84/0xb8
> [ 0.365690] [<800a886c>] smp_call_function_single+0x110/0x200
> [ 0.365703] ---[ end trace 5785856ca39c79d5 ]---
>
> v2->v3:
> No Change.
>
> v3->v4:
> Rebase on top of kernel 5.6-rc1.
>
> drivers/clocksource/ingenic-timer.c | 200
> ++++++++++++++++++++++++------------
> 1 file changed, 134 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/clocksource/ingenic-timer.c
> b/drivers/clocksource/ingenic-timer.c
> index 4bbdb3d..9127c73 100644
> --- a/drivers/clocksource/ingenic-timer.c
> +++ b/drivers/clocksource/ingenic-timer.c
> @@ -1,7 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> - * JZ47xx SoCs TCU IRQ driver
> + * XBurst SoCs TCU IRQ driver
> * Copyright (C) 2019 Paul Cercueil <paul@crapouillou.net>
> + * Copyright (C) 2020 周琰杰 (Zhou Yanjie)
> <zhouyanjie@wanyeetech.com>
> */
>
> #include <linux/bitops.h>
> @@ -21,18 +22,23 @@
>
> #include <dt-bindings/clock/ingenic,tcu.h>
>
> +static DEFINE_PER_CPU(call_single_data_t, ingenic_cevt_csd);
> +
> struct ingenic_soc_info {
> unsigned int num_channels;
> };
>
> struct ingenic_tcu {
> struct regmap *map;
> + struct device_node *np;
> struct clk *timer_clk, *cs_clk;
> + unsigned int timer_local[NR_CPUS];
> unsigned int timer_channel, cs_channel;
> struct clock_event_device cevt;
> struct clocksource cs;
> - char name[4];
> + char name[16];
> unsigned long pwm_channels_mask;
> + int cpu;
> };
>
> static struct ingenic_tcu *ingenic_tcu;
> @@ -81,6 +87,25 @@ static int ingenic_tcu_cevt_set_next(unsigned long
> next,
> return 0;
> }
>
> +static void ingenic_per_cpu_event_handler(void *info)
> +{
> + struct clock_event_device *cevt = (struct clock_event_device *)
> info;
> +
> + if (cevt->event_handler)
> + cevt->event_handler(cevt);
> +}
> +
> +static void ingenic_tcu_per_cpu_cb(struct clock_event_device *evt)
> +{
> + struct ingenic_tcu *tcu = to_ingenic_tcu(evt);
> + call_single_data_t *csd;
> +
> + csd = &per_cpu(ingenic_cevt_csd, tcu->cpu);
> + csd->info = (void *) evt;
> + csd->func = ingenic_per_cpu_event_handler;
> + smp_call_function_single_async(tcu->cpu, csd);
> +}
> +
> static irqreturn_t ingenic_tcu_cevt_cb(int irq, void *dev_id)
> {
> struct clock_event_device *evt = dev_id;
> @@ -88,8 +113,7 @@ static irqreturn_t ingenic_tcu_cevt_cb(int irq,
> void *dev_id)
>
> regmap_write(tcu->map, TCU_REG_TECR, BIT(tcu->timer_channel));
>
> - if (evt->event_handler)
> - evt->event_handler(evt);
> + ingenic_tcu_per_cpu_cb(evt);
I guess the check for (evt->event_handler) is still valid here. There's
no point in calling ingenic_tcu_per_cpu_cb() if in the end the handler
won't do anything.
>
> return IRQ_HANDLED;
> }
> @@ -105,14 +129,73 @@ static struct clk * __init
> ingenic_tcu_get_clock(struct device_node *np, int id)
> return of_clk_get_from_provider(&args);
> }
>
> -static int __init ingenic_tcu_timer_init(struct device_node *np,
> - struct ingenic_tcu *tcu)
> +static int __init ingenic_tcu_clocksource_init(struct device_node
> *np,
> + struct ingenic_tcu *tcu)
Please - don't move functions around. It makes the patch extremely hard
to review. Just add a forward declaration for
ingenic_tcu_clocksource_init() at the top of the file.
> +{
> + unsigned int channel = tcu->cs_channel;
> + struct clocksource *cs = &tcu->cs;
> + unsigned long rate;
> + int err;
> +
> + tcu->cs_clk = ingenic_tcu_get_clock(np, channel);
> + if (IS_ERR(tcu->cs_clk))
> + return PTR_ERR(tcu->cs_clk);
> +
> + err = clk_prepare_enable(tcu->cs_clk);
> + if (err)
> + goto err_clk_put;
> +
> + rate = clk_get_rate(tcu->cs_clk);
> + if (!rate) {
> + err = -EINVAL;
> + goto err_clk_disable;
> + }
> +
> + /* Reset channel */
> + regmap_update_bits(tcu->map, TCU_REG_TCSRc(channel),
> + 0xffff & ~TCU_TCSR_RESERVED_BITS, 0);
> +
> + /* Reset counter */
> + regmap_write(tcu->map, TCU_REG_TDFRc(channel), 0xffff);
> + regmap_write(tcu->map, TCU_REG_TCNTc(channel), 0);
> +
> + /* Enable channel */
> + regmap_write(tcu->map, TCU_REG_TESR, BIT(channel));
> +
> + cs->name = "ingenic-timer";
> + cs->rating = 200;
> + cs->flags = CLOCK_SOURCE_IS_CONTINUOUS;
> + cs->mask = CLOCKSOURCE_MASK(16);
> + cs->read = ingenic_tcu_timer_cs_read;
> +
> + err = clocksource_register_hz(cs, rate);
> + if (err)
> + goto err_clk_disable;
> +
> + return 0;
> +
> +err_clk_disable:
> + clk_disable_unprepare(tcu->cs_clk);
> +err_clk_put:
> + clk_put(tcu->cs_clk);
> + return err;
> +}
> +
> +static int ingenic_tcu_setup_per_cpu_cevt(struct device_node *np,
> + unsigned int channel)
> {
> - unsigned int timer_virq, channel = tcu->timer_channel;
> + unsigned int timer_virq;
> struct irq_domain *domain;
> + struct ingenic_tcu *tcu;
> unsigned long rate;
> int err;
>
> + tcu = kzalloc(sizeof(*tcu), GFP_KERNEL);
> + if (!tcu)
> + return -ENOMEM;
> +
> + tcu->map = ingenic_tcu->map;
> +
> tcu->timer_clk = ingenic_tcu_get_clock(np, channel);
> if (IS_ERR(tcu->timer_clk))
> return PTR_ERR(tcu->timer_clk);
> @@ -139,13 +222,15 @@ static int __init ingenic_tcu_timer_init(struct
> device_node *np,
> goto err_clk_disable;
> }
>
> - snprintf(tcu->name, sizeof(tcu->name), "TCU");
> + snprintf(tcu->name, sizeof(tcu->name), "TCU channel.%u", channel);
Make it TCU%u
>
> err = request_irq(timer_virq, ingenic_tcu_cevt_cb, IRQF_TIMER,
> tcu->name, &tcu->cevt);
> if (err)
> goto err_irq_dispose_mapping;
>
> + tcu->cpu = smp_processor_id();
> + tcu->timer_channel = channel;
> tcu->cevt.cpumask = cpumask_of(smp_processor_id());
> tcu->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
> tcu->cevt.name = tcu->name;
> @@ -166,56 +251,23 @@ static int __init ingenic_tcu_timer_init(struct
> device_node *np,
> return err;
> }
>
> -static int __init ingenic_tcu_clocksource_init(struct device_node
> *np,
> - struct ingenic_tcu *tcu)
> +static int ingenic_tcu_setup_cevt(unsigned int cpu)
> {
> - unsigned int channel = tcu->cs_channel;
> - struct clocksource *cs = &tcu->cs;
> - unsigned long rate;
> - int err;
> -
> - tcu->cs_clk = ingenic_tcu_get_clock(np, channel);
> - if (IS_ERR(tcu->cs_clk))
> - return PTR_ERR(tcu->cs_clk);
> -
> - err = clk_prepare_enable(tcu->cs_clk);
> - if (err)
> - goto err_clk_put;
> -
> - rate = clk_get_rate(tcu->cs_clk);
> - if (!rate) {
> - err = -EINVAL;
> - goto err_clk_disable;
> - }
> -
> - /* Reset channel */
> - regmap_update_bits(tcu->map, TCU_REG_TCSRc(channel),
> - 0xffff & ~TCU_TCSR_RESERVED_BITS, 0);
> -
> - /* Reset counter */
> - regmap_write(tcu->map, TCU_REG_TDFRc(channel), 0xffff);
> - regmap_write(tcu->map, TCU_REG_TCNTc(channel), 0);
> -
> - /* Enable channel */
> - regmap_write(tcu->map, TCU_REG_TESR, BIT(channel));
> -
> - cs->name = "ingenic-timer";
> - cs->rating = 200;
> - cs->flags = CLOCK_SOURCE_IS_CONTINUOUS;
> - cs->mask = CLOCKSOURCE_MASK(16);
> - cs->read = ingenic_tcu_timer_cs_read;
> + int ret;
>
> - err = clocksource_register_hz(cs, rate);
> - if (err)
> - goto err_clk_disable;
> + ret = ingenic_tcu_setup_per_cpu_cevt(ingenic_tcu->np,
> + ingenic_tcu->timer_local[cpu]);
> + if (ret)
> + goto err_tcu_clocksource_cleanup;
>
> return 0;
>
> -err_clk_disable:
> - clk_disable_unprepare(tcu->cs_clk);
> -err_clk_put:
> - clk_put(tcu->cs_clk);
> - return err;
> +err_tcu_clocksource_cleanup:
> + clocksource_unregister(&ingenic_tcu->cs);
> + clk_disable_unprepare(ingenic_tcu->cs_clk);
> + clk_put(ingenic_tcu->cs_clk);
> + kfree(ingenic_tcu);
> + return ret;
> }
>
> static const struct ingenic_soc_info jz4740_soc_info = {
> @@ -239,6 +291,7 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
> const struct ingenic_soc_info *soc_info = id->data;
> struct ingenic_tcu *tcu;
> struct regmap *map;
> + unsigned cpu = 0;
> long rate;
> int ret;
>
> @@ -253,12 +306,14 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
> return -ENOMEM;
>
> /* Enable all TCU channels for PWM use by default except channels
> 0/1 */
> - tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1, 2);
> + tcu->pwm_channels_mask = GENMASK(soc_info->num_channels - 1,
> + NR_CPUS + 1);
> of_property_read_u32(np, "ingenic,pwm-channels-mask",
> (u32 *)&tcu->pwm_channels_mask);
>
> /* Verify that we have at least two free channels */
> - if (hweight8(tcu->pwm_channels_mask) > soc_info->num_channels - 2) {
> + if (hweight8(tcu->pwm_channels_mask) >
> + soc_info->num_channels - NR_CPUS + 1) {
> pr_crit("%s: Invalid PWM channel mask: 0x%02lx\n", __func__,
> tcu->pwm_channels_mask);
> ret = -EINVAL;
> @@ -266,13 +321,29 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
> }
>
> tcu->map = map;
> + tcu->np = np;
> ingenic_tcu = tcu;
>
> - tcu->timer_channel = find_first_zero_bit(&tcu->pwm_channels_mask,
> + tcu->timer_local[cpu] = find_first_zero_bit(&tcu->pwm_channels_mask,
> soc_info->num_channels);
> - tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> - soc_info->num_channels,
> - tcu->timer_channel + 1);
> +
> + if (NR_CPUS > 1) {
> + for (cpu = 1; cpu < NR_CPUS; cpu++)
> + tcu->timer_local[cpu] = find_next_zero_bit(
> + &tcu->pwm_channels_mask,
> + soc_info->num_channels,
> + tcu->timer_local[cpu - 1] + 1);
> +
> + tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> + soc_info->num_channels,
> + tcu->timer_local[cpu-1] + 1);
> + } else {
> + tcu->cs_channel = find_next_zero_bit(&tcu->pwm_channels_mask,
> + soc_info->num_channels,
> + tcu->timer_local[cpu] + 1);
> + }
> +
> +
>
> ret = ingenic_tcu_clocksource_init(np, tcu);
> if (ret) {
> @@ -280,9 +351,10 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
> goto err_free_ingenic_tcu;
> }
>
> - ret = ingenic_tcu_timer_init(np, tcu);
> - if (ret)
> - goto err_tcu_clocksource_cleanup;
> + /* Setup clock events on each CPU core */
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst:
> online",
> + ingenic_tcu_setup_cevt, NULL);
> + WARN_ON(ret < 0);
>
> /* Register the sched_clock at the end as there's no way to undo it
> */
> rate = clk_get_rate(tcu->cs_clk);
> @@ -290,10 +362,6 @@ static int __init ingenic_tcu_init(struct
> device_node *np)
>
> return 0;
>
> -err_tcu_clocksource_cleanup:
> - clocksource_unregister(&tcu->cs);
> - clk_disable_unprepare(tcu->cs_clk);
> - clk_put(tcu->cs_clk);
> err_free_ingenic_tcu:
> kfree(tcu);
> return ret;
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH 0/3] random: add random.rng_seed to bootconfig entry
From: Mark Salyzyn @ 2020-02-14 18:31 UTC (permalink / raw)
To: Rob Herring
Cc: Masami Hiramatsu, linux-kernel@vger.kernel.org,
Android Kernel Team, Theodore Ts'o, Arnd Bergmann,
Greg Kroah-Hartman, Richard Henderson, Mark Brown, Kees Cook,
Hsin-Yi Wang, Vasily Gorbik, Andrew Morton, Steven Rostedt,
Mike Rapoport, Arvind Sankar, Dominik Brodowski, Thomas Gleixner,
Alexander Potapenko, Jonathan Corbet, Mauro Carvalho Chehab,
Josh Poimboeuf, Pawan Gupta, Juergen Gross,
Linux Doc Mailing List
In-Reply-To: <CAL_JsqKb=qBH6QXphEZi7vMS+2K5kNj1riXQiUWma=bidAjN5A@mail.gmail.com>
On 2/14/20 10:14 AM, Rob Herring wrote:
> On Fri, Feb 14, 2020 at 11:00 AM Mark Salyzyn <salyzyn@android.com> wrote:
>> On 2/14/20 5:49 AM, Rob Herring wrote:
>>> On Fri, Feb 14, 2020 at 12:10 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>>>> Hi,
>>>>
>>>> The following series is bootconfig based implementation of
>>>> the rng_seed option patch originally from Mark Salyzyn.
>>>> Note that I removed unrelated command line fixes from this
>>>> series.
>>> Why do we need this? There's already multiple other ways to pass
>>> random seed and this doesn't pass the "too complex for the command
>>> line" argument you had for needing bootconfig.
>>>
>>> Rob
>> Android is the use case I can vouch for. But also KVM.
. . .
> I'm familiar with Cuttlefish somewhat. Guess who got virtio-gpu
> working on Android[1]. :) I assume DT doesn't work for you because you
> need x86 builds, but doesn't QEMU use UEFI in that case which also has
> a mechanism for passing entropy.
IDK, will have to ask the Cuttlefish Team why UEFI not being used, will
get back to you.
>
> To clarify my question: Why do we need random seed in bootconfig
> rather than just the kernel command line? I'm not understanding why
> things changed from your original patch.
Command line was identified as the simplest for them to implement
generically for the x86 and arm64 Cuttlefish instances and hence my
original patch series.
However, it also is limited in the size of the entropy string that can
be provided, so we flipped a coin and decided to accept the bootconfig
mechanism as a viable alternative; that BTW appeared to be simpler to
implement (mainly because rubbing out the entropy command line argument
is not easy).
-- Mark
^ permalink raw reply
* Re: Unknown Postinstall Scriptlet Failing to Defer Boot
From: Andrew Boos @ 2020-02-14 18:30 UTC (permalink / raw)
To: Jeremy A. Puhlman; +Cc: openembedded-core
In-Reply-To: <1b01b37f-ba3b-da34-8e04-81e3f2dd72fb@mvista.com>
[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]
Thanks Jeremy for the hint about update-alternatives. It turns out that
lmbench was installing a binary called "stream" that conflicts with
ImageMagick's own stream. All we had to do was remove lmbench from our
IMAGE_INSTALL and the problem went away. I'm posting the original error for
anyone that is having the same issue.
ERROR: imx8-image-3.0b3-r0 do_rootfs: Postinstall scriptlets of
['imagemagick'] have failed. If the intention is to defer them to first
boot,
then please place them into pkg_postinst_ontarget_${PN} ().
Deferring to first boot via 'exit 1' is no longer supported.
Andrew
On Thu, Feb 13, 2020 at 8:02 PM Jeremy A. Puhlman <jpuhlman@mvista.com>
wrote:
> In these kind of cases, by providing the actual error message, people can
> provide more help.
>
> The recipe inherits update-alternatives and sets a large number of
> alternatives. The alternatives are done in the automatically added post
> install scripts. In all likelihood one of the listed links is wrong, or
> already a symlink or something similar.
> On 2/13/2020 4:49 PM, Andrew Boos wrote:
>
> Hi,
>
> I am working on a bitbake build and I am running into an issue where a
> postinst scriptlet is failing via "exit 1" to defer to first boot and I
> cannot figure out which recipe is causing the issue. Does anybody have any
> tips on identifying which recipe is actually exiting with a failure? The
> error log says that it is in imagemagick but the recipe does not have any
> postinst scripts defined. Also, there are no imagemagick .bbappend files in
> my layers. (
> https://github.com/openembedded/meta-openembedded/blob/sumo/meta-oe/recipes-support/imagemagick/imagemagick_7.0.7.bb
> )
>
> Thanks,
> Andrew
>
>
> <http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient> Virus-free.
> www.avg.com
> <http://www.avg.com/email-signature?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=emailclient>
> <#m_-7702559278781328841_DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
>
>
> --
> Jeremy A. Puhlmanjpuhlman@mvista.com
>
>
--
*Andrew Boos* | Avilution, LLC
(c) 251-895-5509
[-- Attachment #2: Type: text/html, Size: 4788 bytes --]
^ permalink raw reply
* Re: [PATCH 01/19] target/arm: Fix field extract from MVFR[0-2]
From: Philippe Mathieu-Daudé @ 2020-02-14 18:29 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: peter.maydell
In-Reply-To: <20200214181547.21408-2-richard.henderson@linaro.org>
On 2/14/20 7:15 PM, Richard Henderson wrote:
> These registers are 32-bits wide. Cut and paste used FIELD_EX64
> instead of the more proper FIELD_EX32. In practice all this did
> was use an unnecessary 64-bit operation, producing correct results.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/arm/cpu.h | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index e943ffe8a9..28cb2be6fc 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -3415,18 +3415,18 @@ static inline bool isar_feature_aa32_fp16_arith(const ARMISARegisters *id)
> static inline bool isar_feature_aa32_fp_d32(const ARMISARegisters *id)
> {
> /* Return true if D16-D31 are implemented */
> - return FIELD_EX64(id->mvfr0, MVFR0, SIMDREG) >= 2;
> + return FIELD_EX32(id->mvfr0, MVFR0, SIMDREG) >= 2;
> }
>
> static inline bool isar_feature_aa32_fpshvec(const ARMISARegisters *id)
> {
> - return FIELD_EX64(id->mvfr0, MVFR0, FPSHVEC) > 0;
> + return FIELD_EX32(id->mvfr0, MVFR0, FPSHVEC) > 0;
> }
>
> static inline bool isar_feature_aa32_fpdp(const ARMISARegisters *id)
> {
> /* Return true if CPU supports double precision floating point */
> - return FIELD_EX64(id->mvfr0, MVFR0, FPDP) > 0;
> + return FIELD_EX32(id->mvfr0, MVFR0, FPDP) > 0;
> }
>
> /*
> @@ -3436,32 +3436,32 @@ static inline bool isar_feature_aa32_fpdp(const ARMISARegisters *id)
> */
> static inline bool isar_feature_aa32_fp16_spconv(const ARMISARegisters *id)
> {
> - return FIELD_EX64(id->mvfr1, MVFR1, FPHP) > 0;
> + return FIELD_EX32(id->mvfr1, MVFR1, FPHP) > 0;
> }
>
> static inline bool isar_feature_aa32_fp16_dpconv(const ARMISARegisters *id)
> {
> - return FIELD_EX64(id->mvfr1, MVFR1, FPHP) > 1;
> + return FIELD_EX32(id->mvfr1, MVFR1, FPHP) > 1;
> }
>
> static inline bool isar_feature_aa32_vsel(const ARMISARegisters *id)
> {
> - return FIELD_EX64(id->mvfr2, MVFR2, FPMISC) >= 1;
> + return FIELD_EX32(id->mvfr2, MVFR2, FPMISC) >= 1;
> }
>
> static inline bool isar_feature_aa32_vcvt_dr(const ARMISARegisters *id)
> {
> - return FIELD_EX64(id->mvfr2, MVFR2, FPMISC) >= 2;
> + return FIELD_EX32(id->mvfr2, MVFR2, FPMISC) >= 2;
> }
>
> static inline bool isar_feature_aa32_vrint(const ARMISARegisters *id)
> {
> - return FIELD_EX64(id->mvfr2, MVFR2, FPMISC) >= 3;
> + return FIELD_EX32(id->mvfr2, MVFR2, FPMISC) >= 3;
> }
>
> static inline bool isar_feature_aa32_vminmaxnm(const ARMISARegisters *id)
> {
> - return FIELD_EX64(id->mvfr2, MVFR2, FPMISC) >= 4;
> + return FIELD_EX32(id->mvfr2, MVFR2, FPMISC) >= 4;
> }
>
> static inline bool isar_feature_aa32_pan(const ARMISARegisters *id)
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply
* [PATCH AUTOSEL 4.19 017/252] powerpc/powernv/iov: Ensure the pdn for VFs always contains a valid PE number
From: Sasha Levin @ 2020-02-14 16:07 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Alexey Kardashevskiy, Oliver O'Halloran, linuxppc-dev,
Sasha Levin
In-Reply-To: <20200214161147.15842-1-sashal@kernel.org>
From: Oliver O'Halloran <oohall@gmail.com>
[ Upstream commit 3b5b9997b331e77ce967eba2c4bc80dc3134a7fe ]
On pseries there is a bug with adding hotplugged devices to an IOMMU
group. For a number of dumb reasons fixing that bug first requires
re-working how VFs are configured on PowerNV. For background, on
PowerNV we use the pcibios_sriov_enable() hook to do two things:
1. Create a pci_dn structure for each of the VFs, and
2. Configure the PHB's internal BARs so the MMIO range for each VF
maps to a unique PE.
Roughly speaking a PE is the hardware counterpart to a Linux IOMMU
group since all the devices in a PE share the same IOMMU table. A PE
also defines the set of devices that should be isolated in response to
a PCI error (i.e. bad DMA, UR/CA, AER events, etc). When isolated all
MMIO and DMA traffic to and from devicein the PE is blocked by the
root complex until the PE is recovered by the OS.
The requirement to block MMIO causes a giant headache because the P8
PHB generally uses a fixed mapping between MMIO addresses and PEs. As
a result we need to delay configuring the IOMMU groups for device
until after MMIO resources are assigned. For physical devices (i.e.
non-VFs) the PE assignment is done in pcibios_setup_bridge() which is
called immediately after the MMIO resources for downstream
devices (and the bridge's windows) are assigned. For VFs the setup is
more complicated because:
a) pcibios_setup_bridge() is not called again when VFs are activated, and
b) The pci_dev for VFs are created by generic code which runs after
pcibios_sriov_enable() is called.
The work around for this is a two step process:
1. A fixup in pcibios_add_device() is used to initialised the cached
pe_number in pci_dn, then
2. A bus notifier then adds the device to the IOMMU group for the PE
specified in pci_dn->pe_number.
A side effect fixing the pseries bug mentioned in the first paragraph
is moving the fixup out of pcibios_add_device() and into
pcibios_bus_add_device(), which is called much later. This results in
step 2. failing because pci_dn->pe_number won't be initialised when
the bus notifier is run.
We can fix this by removing the need for the fixup. The PE for a VF is
known before the VF is even scanned so we can initialise
pci_dn->pe_number pcibios_sriov_enable() instead. Unfortunately,
moving the initialisation causes two problems:
1. We trip the WARN_ON() in the current fixup code, and
2. The EEH core clears pdn->pe_number when recovering a VF and
relies on the fixup to correctly re-set it.
The only justification for either of these is a comment in
eeh_rmv_device() suggesting that pdn->pe_number *must* be set to
IODA_INVALID_PE in order for the VF to be scanned. However, this
comment appears to have no basis in reality. Both bugs can be fixed by
just deleting the code.
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191028085424.12006-1-oohall@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
arch/powerpc/kernel/eeh_driver.c | 6 ------
arch/powerpc/platforms/powernv/pci-ioda.c | 19 +++++++++++++++----
arch/powerpc/platforms/powernv/pci.c | 4 ----
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index af1f3d5f9a0f7..377d23f58197a 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -554,12 +554,6 @@ static void *eeh_rmv_device(struct eeh_dev *edev, void *userdata)
pci_iov_remove_virtfn(edev->physfn, pdn->vf_index);
edev->pdev = NULL;
-
- /*
- * We have to set the VF PE number to invalid one, which is
- * required to plug the VF successfully.
- */
- pdn->pe_number = IODA_INVALID_PE;
#endif
if (rmv_data)
list_add(&edev->rmv_list, &rmv_data->edev_list);
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index ee63749a2d47e..28adfe4dd04c5 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1552,6 +1552,10 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs)
/* Reserve PE for each VF */
for (vf_index = 0; vf_index < num_vfs; vf_index++) {
+ int vf_devfn = pci_iov_virtfn_devfn(pdev, vf_index);
+ int vf_bus = pci_iov_virtfn_bus(pdev, vf_index);
+ struct pci_dn *vf_pdn;
+
if (pdn->m64_single_mode)
pe_num = pdn->pe_num_map[vf_index];
else
@@ -1564,13 +1568,11 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs)
pe->pbus = NULL;
pe->parent_dev = pdev;
pe->mve_number = -1;
- pe->rid = (pci_iov_virtfn_bus(pdev, vf_index) << 8) |
- pci_iov_virtfn_devfn(pdev, vf_index);
+ pe->rid = (vf_bus << 8) | vf_devfn;
pe_info(pe, "VF %04d:%02d:%02d.%d associated with PE#%x\n",
hose->global_number, pdev->bus->number,
- PCI_SLOT(pci_iov_virtfn_devfn(pdev, vf_index)),
- PCI_FUNC(pci_iov_virtfn_devfn(pdev, vf_index)), pe_num);
+ PCI_SLOT(vf_devfn), PCI_FUNC(vf_devfn), pe_num);
if (pnv_ioda_configure_pe(phb, pe)) {
/* XXX What do we do here ? */
@@ -1584,6 +1586,15 @@ static void pnv_ioda_setup_vf_PE(struct pci_dev *pdev, u16 num_vfs)
list_add_tail(&pe->list, &phb->ioda.pe_list);
mutex_unlock(&phb->ioda.pe_list_mutex);
+ /* associate this pe to it's pdn */
+ list_for_each_entry(vf_pdn, &pdn->parent->child_list, list) {
+ if (vf_pdn->busno == vf_bus &&
+ vf_pdn->devfn == vf_devfn) {
+ vf_pdn->pe_number = pe_num;
+ break;
+ }
+ }
+
pnv_pci_ioda2_setup_dma_pe(phb, pe);
}
}
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index c846300b78368..aa95b8e0f66ad 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -822,16 +822,12 @@ void pnv_pci_dma_dev_setup(struct pci_dev *pdev)
struct pnv_phb *phb = hose->private_data;
#ifdef CONFIG_PCI_IOV
struct pnv_ioda_pe *pe;
- struct pci_dn *pdn;
/* Fix the VF pdn PE number */
if (pdev->is_virtfn) {
- pdn = pci_get_pdn(pdev);
- WARN_ON(pdn->pe_number != IODA_INVALID_PE);
list_for_each_entry(pe, &phb->ioda.pe_list, list) {
if (pe->rid == ((pdev->bus->number << 8) |
(pdev->devfn & 0xff))) {
- pdn->pe_number = pe->pe_number;
pe->pdev = pdev;
break;
}
--
2.20.1
^ permalink raw reply related
* [RFC PATCH v2 2/2] x86/resctrl: Split struct rdt_domain
From: James Morse @ 2020-02-14 18:29 UTC (permalink / raw)
To: x86, linux-kernel
Cc: Fenghua Yu, Reinette Chatre, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, Babu Moger, James Morse
In-Reply-To: <20200214182947.39194-1-james.morse@arm.com>
resctrl is the defacto Linux ABI for SoC resource partitioning features.
To support it on another architecture, we need to abstract it from
Intel RDT, and move it to /fs/.
Split struct rdt_domain up too. Move everything that that is particular
to resctrl into a new header file. resctrl code paths touching a 'hw'
struct indicates where an abstraction is needed.
No change in behaviour, this patch just moves types around.
Signed-off-by: James Morse <james.morse@arm.com>
---
arch/x86/kernel/cpu/resctrl/core.c | 32 +++++++++++-------
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 10 ++++--
arch/x86/kernel/cpu/resctrl/internal.h | 40 +++++------------------
arch/x86/kernel/cpu/resctrl/monitor.c | 8 +++--
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 29 ++++++++++------
include/linux/resctrl.h | 35 +++++++++++++++++++-
6 files changed, 94 insertions(+), 60 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index ce02f3f35b44..e48d54dfd657 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -367,10 +367,11 @@ static void
mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r)
{
unsigned int i;
+ struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d);
struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
for (i = m->low; i < m->high; i++)
- wrmsrl(hw_res->msr_base + i, d->ctrl_val[i]);
+ wrmsrl(hw_res->msr_base + i, hw_dom->ctrl_val[i]);
}
/*
@@ -392,21 +393,23 @@ mba_wrmsr_intel(struct rdt_domain *d, struct msr_param *m,
struct rdt_resource *r)
{
unsigned int i;
+ struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d);
struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
/* Write the delay values for mba. */
for (i = m->low; i < m->high; i++)
- wrmsrl(hw_res->msr_base + i, delay_bw_map(d->ctrl_val[i], r));
+ wrmsrl(hw_res->msr_base + i, delay_bw_map(hw_dom->ctrl_val[i], r));
}
static void
cat_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r)
{
unsigned int i;
+ struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d);
struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
for (i = m->low; i < m->high; i++)
- wrmsrl(hw_res->msr_base + cbm_idx(r, i), d->ctrl_val[i]);
+ wrmsrl(hw_res->msr_base + cbm_idx(r, i), hw_dom->ctrl_val[i]);
}
struct rdt_domain *get_domain_from_cpu(int cpu, struct rdt_resource *r)
@@ -491,21 +494,22 @@ void setup_default_ctrlval(struct rdt_resource *r, u32 *dc, u32 *dm)
static int domain_setup_ctrlval(struct rdt_resource *r, struct rdt_domain *d)
{
struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
+ struct rdt_hw_domain *hw_dom = resctrl_to_arch_dom(d);
struct msr_param m;
u32 *dc, *dm;
- dc = kmalloc_array(r->num_closid, sizeof(*d->ctrl_val), GFP_KERNEL);
+ dc = kmalloc_array(r->num_closid, sizeof(*hw_dom->ctrl_val), GFP_KERNEL);
if (!dc)
return -ENOMEM;
- dm = kmalloc_array(r->num_closid, sizeof(*d->mbps_val), GFP_KERNEL);
+ dm = kmalloc_array(r->num_closid, sizeof(*hw_dom->mbps_val), GFP_KERNEL);
if (!dm) {
kfree(dc);
return -ENOMEM;
}
- d->ctrl_val = dc;
- d->mbps_val = dm;
+ hw_dom->ctrl_val = dc;
+ hw_dom->mbps_val = dm;
setup_default_ctrlval(r, dc, dm);
m.low = 0;
@@ -567,6 +571,7 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r)
{
int id = get_cpu_cacheinfo_id(cpu, r->cache_level);
struct list_head *add_pos = NULL;
+ struct rdt_hw_domain *hw_dom;
struct rdt_domain *d;
d = rdt_find_domain(r, id, &add_pos);
@@ -580,10 +585,11 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r)
return;
}
- d = kzalloc_node(sizeof(*d), GFP_KERNEL, cpu_to_node(cpu));
- if (!d)
+ hw_dom = kzalloc_node(sizeof(*hw_dom), GFP_KERNEL, cpu_to_node(cpu));
+ if (!hw_dom)
return;
+ d = &hw_dom->resctrl;
d->id = id;
cpumask_set_cpu(cpu, &d->cpu_mask);
@@ -610,6 +616,7 @@ static void domain_add_cpu(int cpu, struct rdt_resource *r)
static void domain_remove_cpu(int cpu, struct rdt_resource *r)
{
int id = get_cpu_cacheinfo_id(cpu, r->cache_level);
+ struct rdt_hw_domain *hw_dom;
struct rdt_domain *d;
d = rdt_find_domain(r, id, NULL);
@@ -617,6 +624,7 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r)
pr_warn("Could't find cache id for cpu %d\n", cpu);
return;
}
+ hw_dom = resctrl_to_arch_dom(d);
cpumask_clear_cpu(cpu, &d->cpu_mask);
if (cpumask_empty(&d->cpu_mask)) {
@@ -649,12 +657,12 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r)
if (d->plr)
d->plr->d = NULL;
- kfree(d->ctrl_val);
- kfree(d->mbps_val);
+ kfree(hw_dom->ctrl_val);
+ kfree(hw_dom->mbps_val);
bitmap_free(d->rmid_busy_llc);
kfree(d->mbm_total);
kfree(d->mbm_local);
- kfree(d);
+ kfree(hw_dom);
return;
}
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index c90aa79d90b9..cd79fc3715d3 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -237,6 +237,7 @@ static int parse_line(char *line, struct rdt_resource *r,
int update_domains(struct rdt_resource *r, int closid)
{
+ struct rdt_hw_domain *hw_dom;
struct msr_param msr_param;
cpumask_var_t cpu_mask;
struct rdt_domain *d;
@@ -253,7 +254,8 @@ int update_domains(struct rdt_resource *r, int closid)
mba_sc = is_mba_sc(r);
list_for_each_entry(d, &r->domains, list) {
- dc = !mba_sc ? d->ctrl_val : d->mbps_val;
+ hw_dom = resctrl_to_arch_dom(d);
+ dc = !mba_sc ? hw_dom->ctrl_val : hw_dom->mbps_val;
if (d->have_new_ctrl && d->new_ctrl != dc[closid]) {
cpumask_set_cpu(cpumask_any(&d->cpu_mask), cpu_mask);
dc[closid] = d->new_ctrl;
@@ -372,17 +374,19 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
static void show_doms(struct seq_file *s, struct rdt_resource *r, int closid)
{
+ struct rdt_hw_domain *hw_dom;
struct rdt_domain *dom;
bool sep = false;
u32 ctrl_val;
seq_printf(s, "%*s:", max_name_width, r->name);
list_for_each_entry(dom, &r->domains, list) {
+ hw_dom = resctrl_to_arch_dom(dom);
if (sep)
seq_puts(s, ";");
- ctrl_val = (!is_mba_sc(r) ? dom->ctrl_val[closid] :
- dom->mbps_val[closid]);
+ ctrl_val = (!is_mba_sc(r) ? hw_dom->ctrl_val[closid] :
+ hw_dom->mbps_val[closid]);
seq_printf(s, r->format_str, dom->id, max_data_width,
ctrl_val);
sep = true;
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 5e69f709b729..bc4089a1e775 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -291,44 +291,22 @@ struct mbm_state {
};
/**
- * struct rdt_domain - group of cpus sharing an RDT resource
- * @list: all instances of this resource
- * @id: unique id for this instance
- * @cpu_mask: which cpus share this resource
- * @rmid_busy_llc:
- * bitmap of which limbo RMIDs are above threshold
- * @mbm_total: saved state for MBM total bandwidth
- * @mbm_local: saved state for MBM local bandwidth
- * @mbm_over: worker to periodically read MBM h/w counters
- * @cqm_limbo: worker to periodically read CQM h/w counters
- * @mbm_work_cpu:
- * worker cpu for MBM h/w counters
- * @cqm_work_cpu:
- * worker cpu for CQM h/w counters
+ * struct rdt_hw_domain - group of cpus sharing an RDT resource
+ * @resctrl: Properties exposed to the resctrl file system
* @ctrl_val: array of cache or mem ctrl values (indexed by CLOSID)
* @mbps_val: When mba_sc is enabled, this holds the bandwidth in MBps
- * @new_ctrl: new ctrl value to be loaded
- * @have_new_ctrl: did user provide new_ctrl for this domain
- * @plr: pseudo-locked region (if any) associated with domain
*/
-struct rdt_domain {
- struct list_head list;
- int id;
- struct cpumask cpu_mask;
- unsigned long *rmid_busy_llc;
- struct mbm_state *mbm_total;
- struct mbm_state *mbm_local;
- struct delayed_work mbm_over;
- struct delayed_work cqm_limbo;
- int mbm_work_cpu;
- int cqm_work_cpu;
+struct rdt_hw_domain {
+ struct rdt_domain resctrl;
u32 *ctrl_val;
u32 *mbps_val;
- u32 new_ctrl;
- bool have_new_ctrl;
- struct pseudo_lock_region *plr;
};
+static inline struct rdt_hw_domain *resctrl_to_arch_dom(struct rdt_domain *r)
+{
+ return container_of(r, struct rdt_hw_domain, resctrl);
+}
+
/**
* struct msr_param - set a range of MSRs from a domain
* @res: The resource to use
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index cd34a06cec68..7b3b78c560d8 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -354,6 +354,7 @@ static void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm)
u32 closid, rmid, cur_msr, cur_msr_val, new_msr_val;
struct mbm_state *pmbm_data, *cmbm_data;
struct rdt_hw_resource *hw_r_mba;
+ struct rdt_hw_domain *hw_dom_mba;
u32 cur_bw, delta_bw, user_bw;
struct rdt_resource *r_mba;
struct rdt_domain *dom_mba;
@@ -374,11 +375,12 @@ static void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm)
pr_warn_once("Failure to get domain for MBA update\n");
return;
}
+ hw_dom_mba = resctrl_to_arch_dom(dom_mba);
cur_bw = pmbm_data->prev_bw;
- user_bw = dom_mba->mbps_val[closid];
+ user_bw = hw_dom_mba->mbps_val[closid];
delta_bw = pmbm_data->delta_bw;
- cur_msr_val = dom_mba->ctrl_val[closid];
+ cur_msr_val = hw_dom_mba->ctrl_val[closid];
/*
* For Ctrl groups read data from child monitor groups.
@@ -415,7 +417,7 @@ static void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm)
cur_msr = hw_r_mba->msr_base + closid;
wrmsrl(cur_msr, delay_bw_map(new_msr_val, r_mba));
- dom_mba->ctrl_val[closid] = new_msr_val;
+ hw_dom_mba->ctrl_val[closid] = new_msr_val;
/*
* Delta values are updated dynamically package wise for each
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index f3106dfc4da6..50e1fd13f04a 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -911,7 +911,7 @@ static int rdt_bit_usage_show(struct kernfs_open_file *of,
list_for_each_entry(dom, &r->domains, list) {
if (sep)
seq_putc(seq, ';');
- ctrl = dom->ctrl_val;
+ ctrl = resctrl_to_arch_dom(dom)->ctrl_val;
sw_shareable = 0;
exclusive = 0;
seq_printf(seq, "%d=", dom->id);
@@ -1175,7 +1175,7 @@ static bool __rdtgroup_cbm_overlaps(struct rdt_resource *r, struct rdt_domain *d
}
/* Check for overlap with other resource groups */
- ctrl = d->ctrl_val;
+ ctrl = resctrl_to_arch_dom(d)->ctrl_val;
for (i = 0; i < closids_supported(); i++, ctrl++) {
ctrl_b = *ctrl;
mode = rdtgroup_mode_by_closid(i);
@@ -1244,6 +1244,7 @@ bool rdtgroup_cbm_overlaps(struct rdt_resource *r, struct rdt_domain *d,
*/
static bool rdtgroup_mode_test_exclusive(struct rdtgroup *rdtgrp)
{
+ struct rdt_hw_domain *hw_dom;
int closid = rdtgrp->closid;
struct rdt_resource *r;
bool has_cache = false;
@@ -1254,7 +1255,8 @@ static bool rdtgroup_mode_test_exclusive(struct rdtgroup *rdtgrp)
continue;
has_cache = true;
list_for_each_entry(d, &r->domains, list) {
- if (rdtgroup_cbm_overlaps(r, d, d->ctrl_val[closid],
+ hw_dom = resctrl_to_arch_dom(d);
+ if (rdtgroup_cbm_overlaps(r, d, hw_dom->ctrl_val[closid],
rdtgrp->closid, false)) {
rdt_last_cmd_puts("Schemata overlaps\n");
return false;
@@ -1386,6 +1388,7 @@ unsigned int rdtgroup_cbm_to_size(struct rdt_resource *r,
static int rdtgroup_size_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
+ struct rdt_hw_domain *hw_dom;
struct rdtgroup *rdtgrp;
struct rdt_resource *r;
struct rdt_domain *d;
@@ -1420,14 +1423,15 @@ static int rdtgroup_size_show(struct kernfs_open_file *of,
sep = false;
seq_printf(s, "%*s:", max_name_width, r->name);
list_for_each_entry(d, &r->domains, list) {
+ hw_dom = resctrl_to_arch_dom(d);
if (sep)
seq_putc(s, ';');
if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) {
size = 0;
} else {
ctrl = (!is_mba_sc(r) ?
- d->ctrl_val[rdtgrp->closid] :
- d->mbps_val[rdtgrp->closid]);
+ hw_dom->ctrl_val[rdtgrp->closid] :
+ hw_dom->mbps_val[rdtgrp->closid]);
if (r->rid == RDT_RESOURCE_MBA)
size = ctrl;
else
@@ -1880,6 +1884,7 @@ static int set_cache_qos_cfg(int level, bool enable)
static int set_mba_sc(bool mba_sc)
{
struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_MBA].resctrl;
+ struct rdt_hw_domain *hw_dom;
struct rdt_domain *d;
if (!is_mbm_enabled() || !is_mba_linear() ||
@@ -1887,8 +1892,10 @@ static int set_mba_sc(bool mba_sc)
return -EINVAL;
r->membw.mba_sc = mba_sc;
- list_for_each_entry(d, &r->domains, list)
- setup_default_ctrlval(r, d->ctrl_val, d->mbps_val);
+ list_for_each_entry(d, &r->domains, list) {
+ hw_dom = resctrl_to_arch_dom(d);
+ setup_default_ctrlval(r, hw_dom->ctrl_val, hw_dom->mbps_val);
+ }
return 0;
}
@@ -2207,6 +2214,7 @@ static int rdt_init_fs_context(struct fs_context *fc)
static int reset_all_ctrls(struct rdt_resource *r)
{
+ struct rdt_hw_domain *hw_dom;
struct msr_param msr_param;
cpumask_var_t cpu_mask;
struct rdt_domain *d;
@@ -2225,10 +2233,11 @@ static int reset_all_ctrls(struct rdt_resource *r)
* from each domain to update the MSRs below.
*/
list_for_each_entry(d, &r->domains, list) {
+ hw_dom = resctrl_to_arch_dom(d);
cpumask_set_cpu(cpumask_any(&d->cpu_mask), cpu_mask);
for (i = 0; i < r->num_closid; i++)
- d->ctrl_val[i] = r->default_ctrl;
+ hw_dom->ctrl_val[i] = r->default_ctrl;
}
cpu = get_cpu();
/* Update CBM on this cpu if it's in cpu_mask. */
@@ -2616,7 +2625,7 @@ static int __init_one_rdt_domain(struct rdt_domain *d, struct rdt_resource *r,
d->have_new_ctrl = false;
d->new_ctrl = r->cache.shareable_bits;
used_b = r->cache.shareable_bits;
- ctrl = d->ctrl_val;
+ ctrl = resctrl_to_arch_dom(d)->ctrl_val;
for (i = 0; i < closids_supported(); i++, ctrl++) {
if (closid_allocated(i) && i != closid) {
mode = rdtgroup_mode_by_closid(i);
@@ -2633,7 +2642,7 @@ static int __init_one_rdt_domain(struct rdt_domain *d, struct rdt_resource *r,
* with an exclusive group.
*/
if (d_cdp)
- peer_ctl = d_cdp->ctrl_val[i];
+ peer_ctl = resctrl_to_arch_dom(d_cdp)->ctrl_val[i];
else
peer_ctl = 0;
used_b |= *ctrl | peer_ctl;
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index a8a499c6644b..dbb1a31814a8 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -15,7 +15,40 @@ int proc_resctrl_show(struct seq_file *m,
#endif
-struct rdt_domain;
+/**
+ * struct rdt_domain - group of cpus sharing an RDT resource
+ * @list: all instances of this resource
+ * @id: unique id for this instance
+ * @cpu_mask: which cpus share this resource
+ * @new_ctrl: new ctrl value to be loaded
+ * @have_new_ctrl: did user provide new_ctrl for this domain
+ * @rmid_busy_llc: bitmap of which limbo RMIDs are above threshold
+ * @mbm_total: saved state for MBM total bandwidth
+ * @mbm_local: saved state for MBM local bandwidth
+ * @mbm_over: worker to periodically read MBM h/w counters
+ * @cqm_limbo: worker to periodically read CQM h/w counters
+ * @mbm_work_cpu: worker cpu for MBM h/w counters
+ * @cqm_work_cpu: worker cpu for CQM h/w counters
+ * @plr: pseudo-locked region (if any) associated with domain
+ */
+struct rdt_domain {
+ struct list_head list;
+ int id;
+ struct cpumask cpu_mask;
+
+ u32 new_ctrl;
+ bool have_new_ctrl;
+
+ unsigned long *rmid_busy_llc;
+ struct mbm_state *mbm_total;
+ struct mbm_state *mbm_local;
+ struct delayed_work mbm_over;
+ struct delayed_work cqm_limbo;
+ int mbm_work_cpu;
+ int cqm_work_cpu;
+
+ struct pseudo_lock_region *plr;
+};
/**
* struct resctrl_cache - Cache allocation related data
--
2.24.1
^ permalink raw reply related
* [RFC PATCH v2 1/2] x86/resctrl: Split struct rdt_resource
From: James Morse @ 2020-02-14 18:29 UTC (permalink / raw)
To: x86, linux-kernel
Cc: Fenghua Yu, Reinette Chatre, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, Babu Moger, James Morse
In-Reply-To: <20200214182947.39194-1-james.morse@arm.com>
resctrl is the defacto Linux ABI for SoC resource partitioning features.
To support it on another architecture, we need to abstract it from
Intel RDT, and move it to /fs/.
Lets start by splitting struct rdt_resource, (the name is kept for now
to keep the noise down), and add some type-trickery to keep the foreach
helpers working.
Move everything that that is particular to resctrl into a new header
file, keeping the x86 msr specific stuff where it is. resctrl code
paths touching a 'hw' struct indicates where an abstraction is needed.
We split rdt_domain up in a similar way in the next patch.
No change in behaviour, this patch just moves types around.
Signed-off-by: James Morse <james.morse@arm.com>
---
arch/x86/kernel/cpu/resctrl/core.c | 231 ++++++++++++----------
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 6 +-
arch/x86/kernel/cpu/resctrl/internal.h | 117 +++--------
arch/x86/kernel/cpu/resctrl/monitor.c | 21 +-
arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 4 +-
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 48 ++---
include/linux/resctrl.h | 100 ++++++++++
7 files changed, 299 insertions(+), 228 deletions(-)
diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
index f2968fb6fb9a..ce02f3f35b44 100644
--- a/arch/x86/kernel/cpu/resctrl/core.c
+++ b/arch/x86/kernel/cpu/resctrl/core.c
@@ -57,119 +57,133 @@ static void
mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m,
struct rdt_resource *r);
-#define domain_init(id) LIST_HEAD_INIT(rdt_resources_all[id].domains)
+#define domain_init(id) LIST_HEAD_INIT(rdt_resources_all[id].resctrl.domains)
-struct rdt_resource rdt_resources_all[] = {
+struct rdt_hw_resource rdt_resources_all[] = {
[RDT_RESOURCE_L3] =
{
- .rid = RDT_RESOURCE_L3,
- .name = "L3",
- .domains = domain_init(RDT_RESOURCE_L3),
+ .resctrl = {
+ .rid = RDT_RESOURCE_L3,
+ .name = "L3",
+ .cache_level = 3,
+ .cache = {
+ .min_cbm_bits = 1,
+ .cbm_idx_mult = 1,
+ .cbm_idx_offset = 0,
+ },
+ .domains = domain_init(RDT_RESOURCE_L3),
+ .parse_ctrlval = parse_cbm,
+ .format_str = "%d=%0*x",
+ .fflags = RFTYPE_RES_CACHE,
+ },
.msr_base = MSR_IA32_L3_CBM_BASE,
.msr_update = cat_wrmsr,
- .cache_level = 3,
- .cache = {
- .min_cbm_bits = 1,
- .cbm_idx_mult = 1,
- .cbm_idx_offset = 0,
- },
- .parse_ctrlval = parse_cbm,
- .format_str = "%d=%0*x",
- .fflags = RFTYPE_RES_CACHE,
},
[RDT_RESOURCE_L3DATA] =
{
- .rid = RDT_RESOURCE_L3DATA,
- .name = "L3DATA",
- .domains = domain_init(RDT_RESOURCE_L3DATA),
+ .resctrl = {
+ .rid = RDT_RESOURCE_L3DATA,
+ .name = "L3DATA",
+ .cache_level = 3,
+ .cache = {
+ .min_cbm_bits = 1,
+ .cbm_idx_mult = 2,
+ .cbm_idx_offset = 0,
+ },
+ .domains = domain_init(RDT_RESOURCE_L3DATA),
+ .parse_ctrlval = parse_cbm,
+ .format_str = "%d=%0*x",
+ .fflags = RFTYPE_RES_CACHE,
+ },
.msr_base = MSR_IA32_L3_CBM_BASE,
.msr_update = cat_wrmsr,
- .cache_level = 3,
- .cache = {
- .min_cbm_bits = 1,
- .cbm_idx_mult = 2,
- .cbm_idx_offset = 0,
- },
- .parse_ctrlval = parse_cbm,
- .format_str = "%d=%0*x",
- .fflags = RFTYPE_RES_CACHE,
},
[RDT_RESOURCE_L3CODE] =
{
- .rid = RDT_RESOURCE_L3CODE,
- .name = "L3CODE",
- .domains = domain_init(RDT_RESOURCE_L3CODE),
+ .resctrl = {
+ .rid = RDT_RESOURCE_L3CODE,
+ .name = "L3CODE",
+ .cache_level = 3,
+ .cache = {
+ .min_cbm_bits = 1,
+ .cbm_idx_mult = 2,
+ .cbm_idx_offset = 1,
+ },
+ .domains = domain_init(RDT_RESOURCE_L3CODE),
+ .parse_ctrlval = parse_cbm,
+ .format_str = "%d=%0*x",
+ .fflags = RFTYPE_RES_CACHE,
+ },
.msr_base = MSR_IA32_L3_CBM_BASE,
.msr_update = cat_wrmsr,
- .cache_level = 3,
- .cache = {
- .min_cbm_bits = 1,
- .cbm_idx_mult = 2,
- .cbm_idx_offset = 1,
- },
- .parse_ctrlval = parse_cbm,
- .format_str = "%d=%0*x",
- .fflags = RFTYPE_RES_CACHE,
},
[RDT_RESOURCE_L2] =
{
- .rid = RDT_RESOURCE_L2,
- .name = "L2",
- .domains = domain_init(RDT_RESOURCE_L2),
+ .resctrl = {
+ .rid = RDT_RESOURCE_L2,
+ .name = "L2",
+ .cache_level = 2,
+ .cache = {
+ .min_cbm_bits = 1,
+ .cbm_idx_mult = 1,
+ .cbm_idx_offset = 0,
+ },
+ .domains = domain_init(RDT_RESOURCE_L2),
+ .parse_ctrlval = parse_cbm,
+ .format_str = "%d=%0*x",
+ .fflags = RFTYPE_RES_CACHE,
+ },
.msr_base = MSR_IA32_L2_CBM_BASE,
.msr_update = cat_wrmsr,
- .cache_level = 2,
- .cache = {
- .min_cbm_bits = 1,
- .cbm_idx_mult = 1,
- .cbm_idx_offset = 0,
- },
- .parse_ctrlval = parse_cbm,
- .format_str = "%d=%0*x",
- .fflags = RFTYPE_RES_CACHE,
},
[RDT_RESOURCE_L2DATA] =
{
- .rid = RDT_RESOURCE_L2DATA,
- .name = "L2DATA",
- .domains = domain_init(RDT_RESOURCE_L2DATA),
+ .resctrl = {
+ .rid = RDT_RESOURCE_L2DATA,
+ .name = "L2DATA",
+ .cache_level = 2,
+ .cache = {
+ .min_cbm_bits = 1,
+ .cbm_idx_mult = 2,
+ .cbm_idx_offset = 0,
+ },
+ .domains = domain_init(RDT_RESOURCE_L2DATA),
+ .parse_ctrlval = parse_cbm,
+ .format_str = "%d=%0*x",
+ .fflags = RFTYPE_RES_CACHE,
+ },
.msr_base = MSR_IA32_L2_CBM_BASE,
.msr_update = cat_wrmsr,
- .cache_level = 2,
- .cache = {
- .min_cbm_bits = 1,
- .cbm_idx_mult = 2,
- .cbm_idx_offset = 0,
- },
- .parse_ctrlval = parse_cbm,
- .format_str = "%d=%0*x",
- .fflags = RFTYPE_RES_CACHE,
},
[RDT_RESOURCE_L2CODE] =
{
- .rid = RDT_RESOURCE_L2CODE,
- .name = "L2CODE",
- .domains = domain_init(RDT_RESOURCE_L2CODE),
+ .resctrl = {
+ .rid = RDT_RESOURCE_L2CODE,
+ .name = "L2CODE",
+ .cache_level = 2,
+ .cache = {
+ .min_cbm_bits = 1,
+ .cbm_idx_mult = 2,
+ .cbm_idx_offset = 1,
+ },
+ .domains = domain_init(RDT_RESOURCE_L2CODE),
+ .parse_ctrlval = parse_cbm,
+ .format_str = "%d=%0*x",
+ .fflags = RFTYPE_RES_CACHE,
+ },
.msr_base = MSR_IA32_L2_CBM_BASE,
.msr_update = cat_wrmsr,
- .cache_level = 2,
- .cache = {
- .min_cbm_bits = 1,
- .cbm_idx_mult = 2,
- .cbm_idx_offset = 1,
- },
- .parse_ctrlval = parse_cbm,
- .format_str = "%d=%0*x",
- .fflags = RFTYPE_RES_CACHE,
},
[RDT_RESOURCE_MBA] =
{
- .rid = RDT_RESOURCE_MBA,
- .name = "MB",
- .domains = domain_init(RDT_RESOURCE_MBA),
- .cache_level = 3,
- .format_str = "%d=%*u",
- .fflags = RFTYPE_RES_MB,
+ .resctrl = {
+ .rid = RDT_RESOURCE_MBA,
+ .name = "MB",
+ .cache_level = 3,
+ .domains = domain_init(RDT_RESOURCE_MBA),
+ .format_str = "%d=%*u",
+ .fflags = RFTYPE_RES_MB,
+ },
},
};
@@ -198,7 +212,7 @@ static unsigned int cbm_idx(struct rdt_resource *r, unsigned int closid)
*/
static inline void cache_alloc_hsw_probe(void)
{
- struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3];
+ struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].resctrl;
u32 l, h, max_cbm = BIT_MASK(20) - 1;
if (wrmsr_safe(MSR_IA32_L3_CBM_BASE, max_cbm, 0))
@@ -224,7 +238,7 @@ static inline void cache_alloc_hsw_probe(void)
bool is_mba_sc(struct rdt_resource *r)
{
if (!r)
- return rdt_resources_all[RDT_RESOURCE_MBA].membw.mba_sc;
+ return rdt_resources_all[RDT_RESOURCE_MBA].resctrl.membw.mba_sc;
return r->membw.mba_sc;
}
@@ -321,8 +335,8 @@ static void rdt_get_cache_alloc_cfg(int idx, struct rdt_resource *r)
static void rdt_get_cdp_config(int level, int type)
{
- struct rdt_resource *r_l = &rdt_resources_all[level];
- struct rdt_resource *r = &rdt_resources_all[type];
+ struct rdt_resource *r_l = &rdt_resources_all[level].resctrl;
+ struct rdt_resource *r = &rdt_resources_all[type].resctrl;
r->num_closid = r_l->num_closid / 2;
r->cache.cbm_len = r_l->cache.cbm_len;
@@ -353,9 +367,10 @@ static void
mba_wrmsr_amd(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r)
{
unsigned int i;
+ struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
for (i = m->low; i < m->high; i++)
- wrmsrl(r->msr_base + i, d->ctrl_val[i]);
+ wrmsrl(hw_res->msr_base + i, d->ctrl_val[i]);
}
/*
@@ -377,19 +392,21 @@ mba_wrmsr_intel(struct rdt_domain *d, struct msr_param *m,
struct rdt_resource *r)
{
unsigned int i;
+ struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
/* Write the delay values for mba. */
for (i = m->low; i < m->high; i++)
- wrmsrl(r->msr_base + i, delay_bw_map(d->ctrl_val[i], r));
+ wrmsrl(hw_res->msr_base + i, delay_bw_map(d->ctrl_val[i], r));
}
static void
cat_wrmsr(struct rdt_domain *d, struct msr_param *m, struct rdt_resource *r)
{
unsigned int i;
+ struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
for (i = m->low; i < m->high; i++)
- wrmsrl(r->msr_base + cbm_idx(r, i), d->ctrl_val[i]);
+ wrmsrl(hw_res->msr_base + cbm_idx(r, i), d->ctrl_val[i]);
}
struct rdt_domain *get_domain_from_cpu(int cpu, struct rdt_resource *r)
@@ -408,13 +425,14 @@ struct rdt_domain *get_domain_from_cpu(int cpu, struct rdt_resource *r)
void rdt_ctrl_update(void *arg)
{
struct msr_param *m = arg;
+ struct rdt_hw_resource *hw_res = resctrl_to_arch_res(m->res);
struct rdt_resource *r = m->res;
int cpu = smp_processor_id();
struct rdt_domain *d;
d = get_domain_from_cpu(cpu, r);
if (d) {
- r->msr_update(d, m, r);
+ hw_res->msr_update(d, m, r);
return;
}
pr_warn_once("cpu %d not found in any domain for resource %s\n",
@@ -472,6 +490,7 @@ void setup_default_ctrlval(struct rdt_resource *r, u32 *dc, u32 *dm)
static int domain_setup_ctrlval(struct rdt_resource *r, struct rdt_domain *d)
{
+ struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
struct msr_param m;
u32 *dc, *dm;
@@ -491,7 +510,7 @@ static int domain_setup_ctrlval(struct rdt_resource *r, struct rdt_domain *d)
m.low = 0;
m.high = r->num_closid;
- r->msr_update(d, &m, r);
+ hw_res->msr_update(d, &m, r);
return 0;
}
@@ -639,7 +658,7 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r)
return;
}
- if (r == &rdt_resources_all[RDT_RESOURCE_L3]) {
+ if (r == &rdt_resources_all[RDT_RESOURCE_L3].resctrl) {
if (is_mbm_enabled() && cpu == d->mbm_work_cpu) {
cancel_delayed_work(&d->mbm_over);
mbm_setup_overflow_handler(d, 0);
@@ -815,9 +834,9 @@ static __init bool get_mem_config(void)
return false;
if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
- return __get_mem_config_intel(&rdt_resources_all[RDT_RESOURCE_MBA]);
+ return __get_mem_config_intel(&rdt_resources_all[RDT_RESOURCE_MBA].resctrl);
else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
- return __rdt_get_mem_config_amd(&rdt_resources_all[RDT_RESOURCE_MBA]);
+ return __rdt_get_mem_config_amd(&rdt_resources_all[RDT_RESOURCE_MBA].resctrl);
return false;
}
@@ -833,14 +852,14 @@ static __init bool get_rdt_alloc_resources(void)
return false;
if (rdt_cpu_has(X86_FEATURE_CAT_L3)) {
- rdt_get_cache_alloc_cfg(1, &rdt_resources_all[RDT_RESOURCE_L3]);
+ rdt_get_cache_alloc_cfg(1, &rdt_resources_all[RDT_RESOURCE_L3].resctrl);
if (rdt_cpu_has(X86_FEATURE_CDP_L3))
rdt_get_cdp_l3_config();
ret = true;
}
if (rdt_cpu_has(X86_FEATURE_CAT_L2)) {
/* CPUID 0x10.2 fields are same format at 0x10.1 */
- rdt_get_cache_alloc_cfg(2, &rdt_resources_all[RDT_RESOURCE_L2]);
+ rdt_get_cache_alloc_cfg(2, &rdt_resources_all[RDT_RESOURCE_L2].resctrl);
if (rdt_cpu_has(X86_FEATURE_CDP_L2))
rdt_get_cdp_l2_config();
ret = true;
@@ -864,7 +883,7 @@ static __init bool get_rdt_mon_resources(void)
if (!rdt_mon_features)
return false;
- return !rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3]);
+ return !rdt_get_mon_l3_config(&rdt_resources_all[RDT_RESOURCE_L3].resctrl);
}
static __init void __check_quirks_intel(void)
@@ -898,9 +917,14 @@ static __init bool get_rdt_resources(void)
static __init void rdt_init_res_defs_intel(void)
{
+ struct rdt_hw_resource *hw_res;
struct rdt_resource *r;
+ int i;
+
+ for (i = 0; i < RDT_NUM_RESOURCES; i++) {
+ hw_res = &rdt_resources_all[i];
+ r = &rdt_resources_all[i].resctrl;
- for_each_rdt_resource(r) {
if (r->rid == RDT_RESOURCE_L3 ||
r->rid == RDT_RESOURCE_L3DATA ||
r->rid == RDT_RESOURCE_L3CODE ||
@@ -909,8 +933,8 @@ static __init void rdt_init_res_defs_intel(void)
r->rid == RDT_RESOURCE_L2CODE)
r->cache.arch_has_sparse_bitmaps = false;
else if (r->rid == RDT_RESOURCE_MBA) {
- r->msr_base = MSR_IA32_MBA_THRTL_BASE;
- r->msr_update = mba_wrmsr_intel;
+ hw_res->msr_base = MSR_IA32_MBA_THRTL_BASE;
+ hw_res->msr_update = mba_wrmsr_intel;
r->parse_ctrlval = parse_bw;
}
}
@@ -918,9 +942,14 @@ static __init void rdt_init_res_defs_intel(void)
static __init void rdt_init_res_defs_amd(void)
{
+ struct rdt_hw_resource *hw_res;
struct rdt_resource *r;
+ int i;
+
+ for (i = 0; i < RDT_NUM_RESOURCES; i++) {
+ hw_res = &rdt_resources_all[i];
+ r = &rdt_resources_all[i].resctrl;
- for_each_rdt_resource(r) {
if (r->rid == RDT_RESOURCE_L3 ||
r->rid == RDT_RESOURCE_L3DATA ||
r->rid == RDT_RESOURCE_L3CODE ||
@@ -929,8 +958,8 @@ static __init void rdt_init_res_defs_amd(void)
r->rid == RDT_RESOURCE_L2CODE)
r->cache.arch_has_sparse_bitmaps = true;
else if (r->rid == RDT_RESOURCE_MBA) {
- r->msr_base = MSR_IA32_MBA_BW_BASE;
- r->msr_update = mba_wrmsr_amd;
+ hw_res->msr_base = MSR_IA32_MBA_BW_BASE;
+ hw_res->msr_update = mba_wrmsr_amd;
r->parse_ctrlval = parse_bw;
}
}
diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 38df876feb54..c90aa79d90b9 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -446,6 +446,7 @@ void mon_event_read(struct rmid_read *rr, struct rdt_domain *d,
int rdtgroup_mondata_show(struct seq_file *m, void *arg)
{
struct kernfs_open_file *of = m->private;
+ struct rdt_hw_resource *hw_res;
u32 resid, evtid, domid;
struct rdtgroup *rdtgrp;
struct rdt_resource *r;
@@ -465,7 +466,8 @@ int rdtgroup_mondata_show(struct seq_file *m, void *arg)
domid = md.u.domid;
evtid = md.u.evtid;
- r = &rdt_resources_all[resid];
+ hw_res = &rdt_resources_all[resid];
+ r = &hw_res->resctrl;
d = rdt_find_domain(r, domid, NULL);
if (IS_ERR_OR_NULL(d)) {
ret = -ENOENT;
@@ -479,7 +481,7 @@ int rdtgroup_mondata_show(struct seq_file *m, void *arg)
else if (rr.val & RMID_VAL_UNAVAIL)
seq_puts(m, "Unavailable\n");
else
- seq_printf(m, "%llu\n", rr.val * r->mon_scale);
+ seq_printf(m, "%llu\n", rr.val * hw_res->mon_scale);
out:
rdtgroup_kn_unlock(of->kn);
diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
index 0172a87de814..5e69f709b729 100644
--- a/arch/x86/kernel/cpu/resctrl/internal.h
+++ b/arch/x86/kernel/cpu/resctrl/internal.h
@@ -2,6 +2,7 @@
#ifndef _ASM_X86_RESCTRL_INTERNAL_H
#define _ASM_X86_RESCTRL_INTERNAL_H
+#include <linux/resctrl.h>
#include <linux/sched.h>
#include <linux/kernfs.h>
#include <linux/fs_context.h>
@@ -340,45 +341,6 @@ struct msr_param {
int high;
};
-/**
- * struct rdt_cache - Cache allocation related data
- * @cbm_len: Length of the cache bit mask
- * @min_cbm_bits: Minimum number of consecutive bits to be set
- * @cbm_idx_mult: Multiplier of CBM index
- * @cbm_idx_offset: Offset of CBM index. CBM index is computed by:
- * closid * cbm_idx_multi + cbm_idx_offset
- * in a cache bit mask
- * @shareable_bits: Bitmask of shareable resource with other
- * executing entities
- * @arch_has_sparse_bitmaps: True if a bitmap like f00f is valid.
- */
-struct rdt_cache {
- unsigned int cbm_len;
- unsigned int min_cbm_bits;
- unsigned int cbm_idx_mult;
- unsigned int cbm_idx_offset;
- unsigned int shareable_bits;
- bool arch_has_sparse_bitmaps;
-};
-
-/**
- * struct rdt_membw - Memory bandwidth allocation related data
- * @min_bw: Minimum memory bandwidth percentage user can request
- * @bw_gran: Granularity at which the memory bandwidth is allocated
- * @arch_needs_linear: True if we can't configure non-linear resources
- * @delay_linear: True if memory B/W delay is in linear scale
- * @mba_sc: True if MBA software controller(mba_sc) is enabled
- * @mb_map: Mapping of memory B/W percentage to memory B/W delay
- */
-struct rdt_membw {
- u32 min_bw;
- u32 bw_gran;
- u32 delay_linear;
- bool arch_needs_linear;
- bool mba_sc;
- u32 *mb_map;
-};
-
static inline bool is_llc_occupancy_enabled(void)
{
return (rdt_mon_features & (1 << QOS_L3_OCCUP_EVENT_ID));
@@ -411,55 +373,24 @@ struct rdt_parse_data {
};
/**
- * struct rdt_resource - attributes of an RDT resource
- * @rid: The index of the resource
- * @alloc_enabled: Is allocation enabled on this machine
- * @mon_enabled: Is monitoring enabled for this feature
- * @alloc_capable: Is allocation available on this machine
- * @mon_capable: Is monitor feature available on this machine
- * @name: Name to use in "schemata" file
- * @num_closid: Number of CLOSIDs available
- * @cache_level: Which cache level defines scope of this resource
- * @default_ctrl: Specifies default cache cbm or memory B/W percent.
+ * struct rdt_hw_resource - hw attributes of an RDT resource
* @msr_base: Base MSR address for CBMs
* @msr_update: Function pointer to update QOS MSRs
- * @data_width: Character width of data when displaying
- * @domains: All domains for this resource
- * @cache: Cache allocation related data
- * @format_str: Per resource format string to show domain value
- * @parse_ctrlval: Per resource function pointer to parse control values
- * @evt_list: List of monitoring events
- * @num_rmid: Number of RMIDs available
* @mon_scale: cqm counter * mon_scale = occupancy in bytes
- * @fflags: flags to choose base and info files
*/
-struct rdt_resource {
- int rid;
- bool alloc_enabled;
- bool mon_enabled;
- bool alloc_capable;
- bool mon_capable;
- char *name;
- int num_closid;
- int cache_level;
- u32 default_ctrl;
+struct rdt_hw_resource {
+ struct rdt_resource resctrl;
unsigned int msr_base;
void (*msr_update) (struct rdt_domain *d, struct msr_param *m,
struct rdt_resource *r);
- int data_width;
- struct list_head domains;
- struct rdt_cache cache;
- struct rdt_membw membw;
- const char *format_str;
- int (*parse_ctrlval)(struct rdt_parse_data *data,
- struct rdt_resource *r,
- struct rdt_domain *d);
- struct list_head evt_list;
- int num_rmid;
unsigned int mon_scale;
- unsigned long fflags;
};
+static inline struct rdt_hw_resource *resctrl_to_arch_res(struct rdt_resource *r)
+{
+ return container_of(r, struct rdt_hw_resource, resctrl);
+}
+
int parse_cbm(struct rdt_parse_data *data, struct rdt_resource *r,
struct rdt_domain *d);
int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
@@ -467,7 +398,7 @@ int parse_bw(struct rdt_parse_data *data, struct rdt_resource *r,
extern struct mutex rdtgroup_mutex;
-extern struct rdt_resource rdt_resources_all[];
+extern struct rdt_hw_resource rdt_resources_all[];
extern struct rdtgroup rdtgroup_default;
DECLARE_STATIC_KEY_FALSE(rdt_alloc_enable_key);
@@ -486,33 +417,37 @@ enum {
RDT_NUM_RESOURCES,
};
+static inline struct rdt_resource *resctrl_inc(struct rdt_resource *res)
+{
+ struct rdt_hw_resource *hw_res = resctrl_to_arch_res(res);
+
+ hw_res++;
+ return &hw_res->resctrl;
+}
+
#define for_each_rdt_resource(r) \
- for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
- r++)
+ for (r = &rdt_resources_all[0].resctrl; \
+ r < &rdt_resources_all[RDT_NUM_RESOURCES].resctrl; \
+ r = resctrl_inc(r))
#define for_each_capable_rdt_resource(r) \
- for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
- r++) \
+ for_each_rdt_resource(r) \
if (r->alloc_capable || r->mon_capable)
#define for_each_alloc_capable_rdt_resource(r) \
- for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
- r++) \
+ for_each_rdt_resource(r) \
if (r->alloc_capable)
#define for_each_mon_capable_rdt_resource(r) \
- for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
- r++) \
+ for_each_rdt_resource(r) \
if (r->mon_capable)
#define for_each_alloc_enabled_rdt_resource(r) \
- for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
- r++) \
+ for_each_rdt_resource(r) \
if (r->alloc_enabled)
#define for_each_mon_enabled_rdt_resource(r) \
- for (r = rdt_resources_all; r < rdt_resources_all + RDT_NUM_RESOURCES;\
- r++) \
+ for_each_rdt_resource(r) \
if (r->mon_enabled)
/* CPUID.(EAX=10H, ECX=ResID=1).EAX */
diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index a02a7f886a0a..cd34a06cec68 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -111,7 +111,7 @@ void __check_limbo(struct rdt_domain *d, bool force_free)
struct rdt_resource *r;
u32 crmid = 1, nrmid;
- r = &rdt_resources_all[RDT_RESOURCE_L3];
+ r = &rdt_resources_all[RDT_RESOURCE_L3].resctrl;
/*
* Skip RMID 0 and start from RMID 1 and check all the RMIDs that
@@ -169,7 +169,7 @@ static void add_rmid_to_limbo(struct rmid_entry *entry)
int cpu;
u64 val;
- r = &rdt_resources_all[RDT_RESOURCE_L3];
+ r = &rdt_resources_all[RDT_RESOURCE_L3].resctrl;
entry->busy = 0;
cpu = get_cpu();
@@ -270,7 +270,7 @@ static int __mon_event_count(u32 rmid, struct rmid_read *rr)
*/
static void mbm_bw_count(u32 rmid, struct rmid_read *rr)
{
- struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3];
+ struct rdt_hw_resource *hw_res = &rdt_resources_all[RDT_RESOURCE_L3];
struct mbm_state *m = &rr->d->mbm_local[rmid];
u64 tval, cur_bw, chunks;
@@ -280,7 +280,7 @@ static void mbm_bw_count(u32 rmid, struct rmid_read *rr)
chunks = mbm_overflow_count(m->prev_bw_msr, tval);
m->chunks += chunks;
- cur_bw = (chunks * r->mon_scale) >> 20;
+ cur_bw = (chunks * hw_res->mon_scale) >> 20;
if (m->delta_comp)
m->delta_bw = abs(cur_bw - m->prev_bw);
@@ -353,6 +353,7 @@ static void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm)
{
u32 closid, rmid, cur_msr, cur_msr_val, new_msr_val;
struct mbm_state *pmbm_data, *cmbm_data;
+ struct rdt_hw_resource *hw_r_mba;
u32 cur_bw, delta_bw, user_bw;
struct rdt_resource *r_mba;
struct rdt_domain *dom_mba;
@@ -362,7 +363,8 @@ static void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm)
if (!is_mbm_local_enabled())
return;
- r_mba = &rdt_resources_all[RDT_RESOURCE_MBA];
+ hw_r_mba = &rdt_resources_all[RDT_RESOURCE_MBA];
+ r_mba = &hw_r_mba->resctrl;
closid = rgrp->closid;
rmid = rgrp->mon.rmid;
pmbm_data = &dom_mbm->mbm_local[rmid];
@@ -411,7 +413,7 @@ static void update_mba_bw(struct rdtgroup *rgrp, struct rdt_domain *dom_mbm)
return;
}
- cur_msr = r_mba->msr_base + closid;
+ cur_msr = hw_r_mba->msr_base + closid;
wrmsrl(cur_msr, delay_bw_map(new_msr_val, r_mba));
dom_mba->ctrl_val[closid] = new_msr_val;
@@ -475,7 +477,7 @@ void cqm_handle_limbo(struct work_struct *work)
mutex_lock(&rdtgroup_mutex);
- r = &rdt_resources_all[RDT_RESOURCE_L3];
+ r = &rdt_resources_all[RDT_RESOURCE_L3].resctrl;
d = container_of(work, struct rdt_domain, cqm_limbo.work);
__check_limbo(d, false);
@@ -605,10 +607,11 @@ static void l3_mon_evt_init(struct rdt_resource *r)
int rdt_get_mon_l3_config(struct rdt_resource *r)
{
+ struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
unsigned int cl_size = boot_cpu_data.x86_cache_size;
int ret;
- r->mon_scale = boot_cpu_data.x86_cache_occ_scale;
+ hw_res->mon_scale = boot_cpu_data.x86_cache_occ_scale;
r->num_rmid = boot_cpu_data.x86_cache_max_rmid + 1;
/*
@@ -621,7 +624,7 @@ int rdt_get_mon_l3_config(struct rdt_resource *r)
resctrl_cqm_threshold = cl_size * 1024 / r->num_rmid;
/* h/w works in units of "boot_cpu_data.x86_cache_occ_scale" */
- resctrl_cqm_threshold /= r->mon_scale;
+ resctrl_cqm_threshold /= hw_res->mon_scale;
ret = dom_data_init(r);
if (ret)
diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
index d7623e1b927d..29ace6b60cda 100644
--- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
+++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
@@ -684,8 +684,8 @@ int rdtgroup_locksetup_enter(struct rdtgroup *rdtgrp)
* resource, the portion of cache used by it should be made
* unavailable to all future allocations from both resources.
*/
- if (rdt_resources_all[RDT_RESOURCE_L3DATA].alloc_enabled ||
- rdt_resources_all[RDT_RESOURCE_L2DATA].alloc_enabled) {
+ if (rdt_resources_all[RDT_RESOURCE_L3DATA].resctrl.alloc_enabled ||
+ rdt_resources_all[RDT_RESOURCE_L2DATA].resctrl.alloc_enabled) {
rdt_last_cmd_puts("CDP enabled\n");
return -EINVAL;
}
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index c84b1f355a9a..f3106dfc4da6 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1021,8 +1021,9 @@ static int max_threshold_occ_show(struct kernfs_open_file *of,
struct seq_file *seq, void *v)
{
struct rdt_resource *r = of->kn->parent->priv;
+ struct rdt_hw_resource *hw_res = resctrl_to_arch_res(r);
- seq_printf(seq, "%u\n", resctrl_cqm_threshold * r->mon_scale);
+ seq_printf(seq, "%u\n", resctrl_cqm_threshold * hw_res->mon_scale);
return 0;
}
@@ -1030,7 +1031,7 @@ static int max_threshold_occ_show(struct kernfs_open_file *of,
static ssize_t max_threshold_occ_write(struct kernfs_open_file *of,
char *buf, size_t nbytes, loff_t off)
{
- struct rdt_resource *r = of->kn->parent->priv;
+ struct rdt_hw_resource *hw_res;
unsigned int bytes;
int ret;
@@ -1041,7 +1042,8 @@ static ssize_t max_threshold_occ_write(struct kernfs_open_file *of,
if (bytes > (boot_cpu_data.x86_cache_size * 1024))
return -EINVAL;
- resctrl_cqm_threshold = bytes / r->mon_scale;
+ hw_res = resctrl_to_arch_res(of->kn->parent->priv);
+ resctrl_cqm_threshold = bytes / hw_res->mon_scale;
return nbytes;
}
@@ -1099,16 +1101,16 @@ static int rdt_cdp_peer_get(struct rdt_resource *r, struct rdt_domain *d,
switch (r->rid) {
case RDT_RESOURCE_L3DATA:
- _r_cdp = &rdt_resources_all[RDT_RESOURCE_L3CODE];
+ _r_cdp = &rdt_resources_all[RDT_RESOURCE_L3CODE].resctrl;
break;
case RDT_RESOURCE_L3CODE:
- _r_cdp = &rdt_resources_all[RDT_RESOURCE_L3DATA];
+ _r_cdp = &rdt_resources_all[RDT_RESOURCE_L3DATA].resctrl;
break;
case RDT_RESOURCE_L2DATA:
- _r_cdp = &rdt_resources_all[RDT_RESOURCE_L2CODE];
+ _r_cdp = &rdt_resources_all[RDT_RESOURCE_L2CODE].resctrl;
break;
case RDT_RESOURCE_L2CODE:
- _r_cdp = &rdt_resources_all[RDT_RESOURCE_L2DATA];
+ _r_cdp = &rdt_resources_all[RDT_RESOURCE_L2DATA].resctrl;
break;
default:
ret = -ENOENT;
@@ -1830,7 +1832,7 @@ static void l2_qos_cfg_update(void *arg)
static inline bool is_mba_linear(void)
{
- return rdt_resources_all[RDT_RESOURCE_MBA].membw.delay_linear;
+ return rdt_resources_all[RDT_RESOURCE_MBA].resctrl.membw.delay_linear;
}
static int set_cache_qos_cfg(int level, bool enable)
@@ -1851,7 +1853,7 @@ static int set_cache_qos_cfg(int level, bool enable)
if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL))
return -ENOMEM;
- r_l = &rdt_resources_all[level];
+ r_l = &rdt_resources_all[level].resctrl;
list_for_each_entry(d, &r_l->domains, list) {
/* Pick one CPU from each domain instance to update MSR */
cpumask_set_cpu(cpumask_any(&d->cpu_mask), cpu_mask);
@@ -1877,7 +1879,7 @@ static int set_cache_qos_cfg(int level, bool enable)
*/
static int set_mba_sc(bool mba_sc)
{
- struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_MBA];
+ struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_MBA].resctrl;
struct rdt_domain *d;
if (!is_mbm_enabled() || !is_mba_linear() ||
@@ -1893,9 +1895,9 @@ static int set_mba_sc(bool mba_sc)
static int cdp_enable(int level, int data_type, int code_type)
{
- struct rdt_resource *r_ldata = &rdt_resources_all[data_type];
- struct rdt_resource *r_lcode = &rdt_resources_all[code_type];
- struct rdt_resource *r_l = &rdt_resources_all[level];
+ struct rdt_resource *r_ldata = &rdt_resources_all[data_type].resctrl;
+ struct rdt_resource *r_lcode = &rdt_resources_all[code_type].resctrl;
+ struct rdt_resource *r_l = &rdt_resources_all[level].resctrl;
int ret;
if (!r_l->alloc_capable || !r_ldata->alloc_capable ||
@@ -1925,13 +1927,13 @@ static int cdpl2_enable(void)
static void cdp_disable(int level, int data_type, int code_type)
{
- struct rdt_resource *r = &rdt_resources_all[level];
+ struct rdt_resource *r = &rdt_resources_all[level].resctrl;
r->alloc_enabled = r->alloc_capable;
- if (rdt_resources_all[data_type].alloc_enabled) {
- rdt_resources_all[data_type].alloc_enabled = false;
- rdt_resources_all[code_type].alloc_enabled = false;
+ if (rdt_resources_all[data_type].resctrl.alloc_enabled) {
+ rdt_resources_all[data_type].resctrl.alloc_enabled = false;
+ rdt_resources_all[code_type].resctrl.alloc_enabled = false;
set_cache_qos_cfg(level, false);
}
}
@@ -1948,9 +1950,9 @@ static void cdpl2_disable(void)
static void cdp_disable_all(void)
{
- if (rdt_resources_all[RDT_RESOURCE_L3DATA].alloc_enabled)
+ if (rdt_resources_all[RDT_RESOURCE_L3DATA].resctrl.alloc_enabled)
cdpl3_disable();
- if (rdt_resources_all[RDT_RESOURCE_L2DATA].alloc_enabled)
+ if (rdt_resources_all[RDT_RESOURCE_L2DATA].resctrl.alloc_enabled)
cdpl2_disable();
}
@@ -2101,7 +2103,7 @@ static int rdt_get_tree(struct fs_context *fc)
static_branch_enable_cpuslocked(&rdt_enable_key);
if (is_mbm_enabled()) {
- r = &rdt_resources_all[RDT_RESOURCE_L3];
+ r = &rdt_resources_all[RDT_RESOURCE_L3].resctrl;
list_for_each_entry(dom, &r->domains, list)
mbm_setup_overflow_handler(dom, MBM_OVERFLOW_INTERVAL);
}
@@ -3092,13 +3094,13 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
static int rdtgroup_show_options(struct seq_file *seq, struct kernfs_root *kf)
{
- if (rdt_resources_all[RDT_RESOURCE_L3DATA].alloc_enabled)
+ if (rdt_resources_all[RDT_RESOURCE_L3DATA].resctrl.alloc_enabled)
seq_puts(seq, ",cdp");
- if (rdt_resources_all[RDT_RESOURCE_L2DATA].alloc_enabled)
+ if (rdt_resources_all[RDT_RESOURCE_L2DATA].resctrl.alloc_enabled)
seq_puts(seq, ",cdpl2");
- if (is_mba_sc(&rdt_resources_all[RDT_RESOURCE_MBA]))
+ if (is_mba_sc(&rdt_resources_all[RDT_RESOURCE_MBA].resctrl))
seq_puts(seq, ",mba_MBps");
return 0;
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 9b05af9b3e28..a8a499c6644b 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -2,6 +2,8 @@
#ifndef _RESCTRL_H
#define _RESCTRL_H
+#include <linux/kernel.h>
+#include <linux/list.h>
#include <linux/pid.h>
#ifdef CONFIG_PROC_CPU_RESCTRL
@@ -13,4 +15,102 @@ int proc_resctrl_show(struct seq_file *m,
#endif
+struct rdt_domain;
+
+/**
+ * struct resctrl_cache - Cache allocation related data
+ * @cbm_len: Length of the cache bit mask
+ * @min_cbm_bits: Minimum number of consecutive bits to be set
+ * @cbm_idx_mult: Multiplier of CBM index
+ * @cbm_idx_offset: Offset of CBM index. CBM index is computed by:
+ * closid * cbm_idx_multi + cbm_idx_offset
+ * in a cache bit mask
+ * @shareable_bits: Bitmask of shareable resource with other
+ * executing entities
+ * @arch_has_sparse_bitmaps: True if a bitmap like f00f is valid.
+ */
+struct resctrl_cache {
+ u32 cbm_len;
+ u32 min_cbm_bits;
+ unsigned int cbm_idx_mult;
+ unsigned int cbm_idx_offset;
+ u32 shareable_bits;
+ bool arch_has_sparse_bitmaps;
+};
+
+/**
+ * struct resctrl_membw - Memory bandwidth allocation related data
+ * @min_bw: Minimum memory bandwidth percentage user can request
+ * @bw_gran: Granularity at which the memory bandwidth is allocated
+ * @delay_linear: True if memory B/W delay is in linear scale
+ * @arch_needs_linear: True if we can't configure non-linear resources
+ * @mba_sc: True if MBA software controller(mba_sc) is enabled
+ * @mb_map: Mapping of memory B/W percentage to memory B/W delay
+ */
+struct resctrl_membw {
+ u32 min_bw;
+ u32 bw_gran;
+ u32 delay_linear;
+ bool arch_needs_linear;
+ bool mba_sc;
+ u32 *mb_map;
+};
+
+struct rdt_parse_data;
+
+/**
+ * @rid: The index of the resource
+ * @alloc_enabled: Is allocation enabled on this machine
+ * @mon_enabled: Is monitoring enabled for this feature
+ * @alloc_capable: Is allocation available on this machine
+ * @mon_capable: Is monitor feature available on this machine
+ *
+ * @cache_level: Which cache level defines scope of this resource
+ *
+ * @cache: If the component has cache controls, their properties.
+ * @membw: If the component has bandwidth controls, their properties.
+ *
+ * @num_closid: Number of CLOSIDs available.
+ * @num_rmid: Number of RMIDs available.
+ *
+ * @domains: All domains for this resource
+ *
+ * @name: Name to use in "schemata" file.
+ * @data_width: Character width of data when displaying.
+ * @default_ctrl: Specifies default cache cbm or memory B/W percent.
+ * @format_str: Per resource format string to show domain value
+ * @parse_ctrlval: Per resource function pointer to parse control values
+ *
+ * @evt_list: List of monitoring events
+ * @fflags: flags to choose base and info files
+ */
+struct rdt_resource {
+ int rid;
+ bool alloc_enabled;
+ bool mon_enabled;
+ bool alloc_capable;
+ bool mon_capable;
+
+ int cache_level;
+
+ struct resctrl_cache cache;
+ struct resctrl_membw membw;
+
+ int num_closid;
+ int num_rmid;
+
+ struct list_head domains;
+
+ char *name;
+ int data_width;
+ u32 default_ctrl;
+ const char *format_str;
+ int (*parse_ctrlval)(struct rdt_parse_data *data,
+ struct rdt_resource *r,
+ struct rdt_domain *d);
+ struct list_head evt_list;
+ unsigned long fflags;
+
+};
+
#endif /* _RESCTRL_H */
--
2.24.1
^ permalink raw reply related
* [RFC PATCH v2 0/2] x86/resctrl: Start abstraction for a second arch
From: James Morse @ 2020-02-14 18:29 UTC (permalink / raw)
To: x86, linux-kernel
Cc: Fenghua Yu, Reinette Chatre, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H . Peter Anvin, Babu Moger, James Morse
Hi folks,
These two patches are the tip of the MPAM iceberg.
Arm have some CPU support for dividing caches into portions, and
applying bandwidth limits at various points in the SoC. The collective term
for these features is MPAM: Memory Partitioning and Monitoring.
MPAM is similar enough to Intel RDT, that it should use the defacto linux
interface: resctrl. This filesystem currently lives under arch/x86, and is
tightly coupled to the architecture.
Ultimately, my plan is to split the existing resctrl code up to have an
arch<->fs abstraction, then move all the bits out to fs/resctrl. From there
MPAM can be wired up.
These two patches are step one: Split the two structs that resctrl uses
to have an arch<->fs split. These sit on top of the cleanup posted here:
lore.kernel.org/r/20200214182401.39008-1-james.morse@arm.com
I'm after strong opinions like "No! struct mbm_state is obviously arch
specific.". Making the hardware configuration belong to the arch code
instead of resctrl is so that it can be scaled on arm64, where MPAM
allows terrifyingly large portion bitmaps for the caches.
Last time these were posted, the request was for the spec, and to see
the whole fully assembled iceberg.
The spec is here:
https://static.docs.arm.com/ddi0598/ab/DDI0598A_b_MPAM_supp_armv8a.pdf
For a slightly dated view of the whole tree:
1. Don peril sensitive sunglasses
2. https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/log/?h=mpam/snapshot/feb
The tree is generally RFC-quality. It gets more ragged once you get out of
the x86 code. I anticipate all the arm64 code being rewritten before its
considered for merging.
(I haven't reposted the CDP origami as before, as I think that series
will be clearer if I re-order the patches ... it may even be shorter)
Does it all work? Almost. Monitor groups are proving to be a problem, I
can't see a way of getting these working without a user-visible change of
behaviour.
MPAMs counters aren't 1:1 with RMIDs, so supporting MBM_* on
any likely hardware will have to be via something other than resctrl.
Thanks,
James Morse (2):
x86/resctrl: Split struct rdt_resource
x86/resctrl: Split struct rdt_domain
arch/x86/kernel/cpu/resctrl/core.c | 257 +++++++++++++---------
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 16 +-
arch/x86/kernel/cpu/resctrl/internal.h | 157 +++----------
arch/x86/kernel/cpu/resctrl/monitor.c | 29 ++-
arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 4 +-
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 77 ++++---
include/linux/resctrl.h | 133 +++++++++++
7 files changed, 389 insertions(+), 284 deletions(-)
--
2.24.1
^ permalink raw reply
* RE: [PATCH] drm/amd/display: Don't take the address of skip_scdc_overwrite in dc_link_detect_helper
From: Liu, Zhan @ 2020-02-14 18:29 UTC (permalink / raw)
To: 'Nathan Chancellor', Wentland, Harry, Li, Sun peng (Leo),
Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing)
Cc: 'clang-built-linux@googlegroups.com',
'dri-devel@lists.freedesktop.org',
'amd-gfx@lists.freedesktop.org',
'linux-kernel@vger.kernel.org'
In-Reply-To: <DM5PR1201MB2554F94D7E206B7690C208109E150@DM5PR1201MB2554.namprd12.prod.outlook.com>
> -----Original Message-----
> From: Liu, Zhan
> Sent: 2020/February/14, Friday 11:01 AM
> To: Nathan Chancellor <natechancellor@gmail.com>; Wentland, Harry
> <Harry.Wentland@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>;
> Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
> <David1.Zhou@amd.com>
> Cc: clang-built-linux@googlegroups.com; dri-devel@lists.freedesktop.org;
> amd-gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] drm/amd/display: Don't take the address of
> skip_scdc_overwrite in dc_link_detect_helper
>
>
>
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> > Nathan Chancellor
> > Sent: 2020/February/14, Friday 1:30 AM
> > To: Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo)
> > <Sunpeng.Li@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>;
> > Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
> > <David1.Zhou@amd.com>
> > Cc: clang-built-linux@googlegroups.com; Nathan Chancellor
> > <natechancellor@gmail.com>; dri-devel@lists.freedesktop.org; amd-
> > gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org
> > Subject: [PATCH] drm/amd/display: Don't take the address of
> > skip_scdc_overwrite in dc_link_detect_helper
> >
> > Clang warns:
> >
> > ../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:980:36:
> > warning: address of 'sink->edid_caps.panel_patch.skip_scdc_overwrite'
> > will always evaluate to 'true' [-Wpointer-bool-conversion]
> > if (&sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > ~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> > 1 warning generated.
> >
> > This is probably not what was intended so remove the address of
> > operator, which matches how skip_scdc_overwrite is handled in the rest of
> the driver.
> >
> > While we're here, drop an extra newline after this if block.
> >
> > Fixes: a760fc1bff03 ("drm/amd/display: add monitor patch to disable
> > SCDC
> > read/write")
> > Link:
> > https://github.com/ClangBuiltLinux/linux/issues/879
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>
> Thank you!
> Reviewed-by: Zhan Liu <zhan.liu@amd.com>
Also applied, thanks!
Zhan
>
> > ---
> >
> > As an aside, I don't see skip_scdc_overwrite assigned a value
> > anywhere, is this working as intended?
> >
> > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index 24d99849be5e..a3bfa05c545e 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -977,10 +977,9 @@ static bool dc_link_detect_helper(struct dc_link
> > *link,
> > if ((prev_sink != NULL) && ((edid_status == EDID_THE_SAME)
> > || (edid_status == EDID_OK)))
> > same_edid = is_same_edid(&prev_sink->dc_edid,
> &sink->dc_edid);
> >
> > - if (&sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > + if (sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > link->ctx->dc->debug.hdmi20_disable = true;
> >
> > -
> > if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT
> &&
> > sink_caps.transaction_type ==
> > DDC_TRANSACTION_TYPE_I2C_OVER_AUX) {
> > /*
> > --
> > 2.25.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
^ permalink raw reply
* RE: [PATCH] drm/amd/display: Don't take the address of skip_scdc_overwrite in dc_link_detect_helper
From: Liu, Zhan @ 2020-02-14 18:29 UTC (permalink / raw)
To: 'Nathan Chancellor', Wentland, Harry, Li, Sun peng (Leo),
Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing)
Cc: 'clang-built-linux@googlegroups.com',
'amd-gfx@lists.freedesktop.org',
'dri-devel@lists.freedesktop.org',
'linux-kernel@vger.kernel.org'
In-Reply-To: <DM5PR1201MB2554F94D7E206B7690C208109E150@DM5PR1201MB2554.namprd12.prod.outlook.com>
> -----Original Message-----
> From: Liu, Zhan
> Sent: 2020/February/14, Friday 11:01 AM
> To: Nathan Chancellor <natechancellor@gmail.com>; Wentland, Harry
> <Harry.Wentland@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>;
> Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
> <David1.Zhou@amd.com>
> Cc: clang-built-linux@googlegroups.com; dri-devel@lists.freedesktop.org;
> amd-gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] drm/amd/display: Don't take the address of
> skip_scdc_overwrite in dc_link_detect_helper
>
>
>
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> > Nathan Chancellor
> > Sent: 2020/February/14, Friday 1:30 AM
> > To: Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo)
> > <Sunpeng.Li@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>;
> > Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
> > <David1.Zhou@amd.com>
> > Cc: clang-built-linux@googlegroups.com; Nathan Chancellor
> > <natechancellor@gmail.com>; dri-devel@lists.freedesktop.org; amd-
> > gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org
> > Subject: [PATCH] drm/amd/display: Don't take the address of
> > skip_scdc_overwrite in dc_link_detect_helper
> >
> > Clang warns:
> >
> > ../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:980:36:
> > warning: address of 'sink->edid_caps.panel_patch.skip_scdc_overwrite'
> > will always evaluate to 'true' [-Wpointer-bool-conversion]
> > if (&sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > ~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> > 1 warning generated.
> >
> > This is probably not what was intended so remove the address of
> > operator, which matches how skip_scdc_overwrite is handled in the rest of
> the driver.
> >
> > While we're here, drop an extra newline after this if block.
> >
> > Fixes: a760fc1bff03 ("drm/amd/display: add monitor patch to disable
> > SCDC
> > read/write")
> > Link:
> > https://github.com/ClangBuiltLinux/linux/issues/879
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>
> Thank you!
> Reviewed-by: Zhan Liu <zhan.liu@amd.com>
Also applied, thanks!
Zhan
>
> > ---
> >
> > As an aside, I don't see skip_scdc_overwrite assigned a value
> > anywhere, is this working as intended?
> >
> > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index 24d99849be5e..a3bfa05c545e 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -977,10 +977,9 @@ static bool dc_link_detect_helper(struct dc_link
> > *link,
> > if ((prev_sink != NULL) && ((edid_status == EDID_THE_SAME)
> > || (edid_status == EDID_OK)))
> > same_edid = is_same_edid(&prev_sink->dc_edid,
> &sink->dc_edid);
> >
> > - if (&sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > + if (sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > link->ctx->dc->debug.hdmi20_disable = true;
> >
> > -
> > if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT
> &&
> > sink_caps.transaction_type ==
> > DDC_TRANSACTION_TYPE_I2C_OVER_AUX) {
> > /*
> > --
> > 2.25.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* RE: [PATCH] drm/amd/display: Don't take the address of skip_scdc_overwrite in dc_link_detect_helper
From: Liu, Zhan @ 2020-02-14 18:29 UTC (permalink / raw)
To: 'Nathan Chancellor', Wentland, Harry, Li, Sun peng (Leo),
Deucher, Alexander, Koenig, Christian, Zhou, David(ChunMing)
Cc: 'clang-built-linux@googlegroups.com',
'amd-gfx@lists.freedesktop.org',
'dri-devel@lists.freedesktop.org',
'linux-kernel@vger.kernel.org'
In-Reply-To: <DM5PR1201MB2554F94D7E206B7690C208109E150@DM5PR1201MB2554.namprd12.prod.outlook.com>
> -----Original Message-----
> From: Liu, Zhan
> Sent: 2020/February/14, Friday 11:01 AM
> To: Nathan Chancellor <natechancellor@gmail.com>; Wentland, Harry
> <Harry.Wentland@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>;
> Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
> <David1.Zhou@amd.com>
> Cc: clang-built-linux@googlegroups.com; dri-devel@lists.freedesktop.org;
> amd-gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] drm/amd/display: Don't take the address of
> skip_scdc_overwrite in dc_link_detect_helper
>
>
>
> > -----Original Message-----
> > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of
> > Nathan Chancellor
> > Sent: 2020/February/14, Friday 1:30 AM
> > To: Wentland, Harry <Harry.Wentland@amd.com>; Li, Sun peng (Leo)
> > <Sunpeng.Li@amd.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>;
> > Koenig, Christian <Christian.Koenig@amd.com>; Zhou, David(ChunMing)
> > <David1.Zhou@amd.com>
> > Cc: clang-built-linux@googlegroups.com; Nathan Chancellor
> > <natechancellor@gmail.com>; dri-devel@lists.freedesktop.org; amd-
> > gfx@lists.freedesktop.org; linux-kernel@vger.kernel.org
> > Subject: [PATCH] drm/amd/display: Don't take the address of
> > skip_scdc_overwrite in dc_link_detect_helper
> >
> > Clang warns:
> >
> > ../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:980:36:
> > warning: address of 'sink->edid_caps.panel_patch.skip_scdc_overwrite'
> > will always evaluate to 'true' [-Wpointer-bool-conversion]
> > if (&sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > ~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> > 1 warning generated.
> >
> > This is probably not what was intended so remove the address of
> > operator, which matches how skip_scdc_overwrite is handled in the rest of
> the driver.
> >
> > While we're here, drop an extra newline after this if block.
> >
> > Fixes: a760fc1bff03 ("drm/amd/display: add monitor patch to disable
> > SCDC
> > read/write")
> > Link:
> > https://github.com/ClangBuiltLinux/linux/issues/879
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>
> Thank you!
> Reviewed-by: Zhan Liu <zhan.liu@amd.com>
Also applied, thanks!
Zhan
>
> > ---
> >
> > As an aside, I don't see skip_scdc_overwrite assigned a value
> > anywhere, is this working as intended?
> >
> > drivers/gpu/drm/amd/display/dc/core/dc_link.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index 24d99849be5e..a3bfa05c545e 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -977,10 +977,9 @@ static bool dc_link_detect_helper(struct dc_link
> > *link,
> > if ((prev_sink != NULL) && ((edid_status == EDID_THE_SAME)
> > || (edid_status == EDID_OK)))
> > same_edid = is_same_edid(&prev_sink->dc_edid,
> &sink->dc_edid);
> >
> > - if (&sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > + if (sink->edid_caps.panel_patch.skip_scdc_overwrite)
> > link->ctx->dc->debug.hdmi20_disable = true;
> >
> > -
> > if (link->connector_signal == SIGNAL_TYPE_DISPLAY_PORT
> &&
> > sink_caps.transaction_type ==
> > DDC_TRANSACTION_TYPE_I2C_OVER_AUX) {
> > /*
> > --
> > 2.25.0
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v1 2/4] Acceptance test: EXEC migration
From: Philippe Mathieu-Daudé @ 2020-02-14 18:27 UTC (permalink / raw)
To: Oksana Vohchana, qemu-devel; +Cc: wainersm, crosa
In-Reply-To: <20200214145235.4378-3-ovoshcha@redhat.com>
On 2/14/20 3:52 PM, Oksana Vohchana wrote:
> Improves EXEC migration to run whole test stage
>
> Signed-off-by: Oksana Vohchana <ovoshcha@redhat.com>
Fixes: 2e768cb682bf
> ---
> tests/acceptance/migration.py | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/acceptance/migration.py b/tests/acceptance/migration.py
> index e4c39b85a1..8209dcf71d 100644
> --- a/tests/acceptance/migration.py
> +++ b/tests/acceptance/migration.py
> @@ -75,3 +75,5 @@ class Migration(Test):
> """
> free_port = self._get_free_port()
> dest_uri = 'exec:nc -l localhost %u' % free_port
> + src_uri = 'exec:nc localhost %u' % free_port
> + self.do_migrate(dest_uri, src_uri)
>
I was very surprised because commit 2e768cb682bf is not the content I
reviewed. Apparently what happened here is you sent 2 different series
with the same subject... I got confused while applying I selected the
incorrect id:
$ pwclient list 'Acceptance test: provides to use different transport
for migration'
ID State Name
-- ----- ----
11348377 New Acceptance test: provides to use different
transport for migration
11348719 New Acceptance test: provides to use different
transport for migration
11355797 New [v2] Acceptance test: provides to use different
transport for migration
11358447 New [v2,REPOST] Acceptance test: provides to use
different transport for migration
11362397 New [v3,1/2] Acceptance test: provides to use
different transport for migration
11362399 New [v3,2/2] Acceptance test: provides to use
different transport for migration
11362479 New [v3,1/2] Acceptance test: provides to use
different transport for migration
11362485 New [v3,2/2] Acceptance test: provides to use
different transport for migration
I am sorry I missed that.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply
* Re: [Intel-gfx] [PATCH] drm/i915/selftests: Fix selftest_mocs for DGFX
From: Chris Wilson @ 2020-02-14 18:29 UTC (permalink / raw)
To: Brian Welty, Daniele Ceraolo Spurio, intel-gfx
In-Reply-To: <383e75e2-a29a-4c8c-e3ba-579008464a45@intel.com>
Quoting Daniele Ceraolo Spurio (2020-02-14 17:56:58)
>
>
> On 2/12/20 4:49 PM, Brian Welty wrote:
> >
> > On 2/12/2020 4:34 PM, Chris Wilson wrote:
> >> Quoting Brian Welty (2020-02-13 00:14:18)
> >>> For DGFX devices, the MOCS control value is not initialized or used.
> >>
> >> Then why is the table populated?
> >> -Chris
> >>
> >
> > The format has changed (been reduced?) for DGFX. drm_i915_mocs_entry.l3cc_value is what is still initialized/used.
> > Probably first needed is the patch that defines the table entries for DGFX.
> > Ugh, I didn't notice this wasn't applied yet. Let me ask about this.
> >
>
> We do have:
>
> commit e6e2ac07118b15f25683fcbd59ea1be73ec9465d
> Author: Lucas De Marchi <lucas.demarchi@intel.com>
> Date: Thu Oct 24 12:51:21 2019 -0700
>
> drm/i915: do not set MOCS control values on dgfx
>
> So I see no reason not to add this change to the test side to match
> that. Maybe we can add an additional check in the test to validate that
> all the control_entries are set to 0 in the table on DGFX?
My expectation was that as we were not setting mocs values, we would not
have defined a table for it. However, the table is combined for mocs and
l3cc. l3cc is still used, right?
My ideal would be that our tables did remain the truth value we could
use directly -- that would require splitting the tables though.
If we did something like
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index de1f83100fb6..2c636257f12c 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -12,7 +12,8 @@
#include "selftests/igt_spinner.h"
struct live_mocs {
- struct drm_i915_mocs_table table;
+ struct drm_i915_mocs_table mocs;
+ struct drm_i915_mocs_table l3cc;
struct i915_vma *scratch;
void *vaddr;
};
@@ -68,13 +69,32 @@ static struct i915_vma *create_scratch(struct intel_gt *gt)
return vma;
}
+static bool has_l3cc(struct drm_i915_private *i915)
+{
+ return true;
+}
+
+static bool has_mocs(struct drm_i915_private *i915)
+{
+ return !IS_DGFX(i915);
+}
+
static int live_mocs_init(struct live_mocs *arg, struct intel_gt *gt)
{
+ struct drm_i915_mocs_table table;
int err;
- if (!get_mocs_settings(gt->i915, &arg->table))
+ memset(arg, 0, sizeof(*arg));
+
+ if (!get_mocs_settings(gt->i915, &table))
return -EINVAL;
+ if (has_l3cc(gt->i915))
+ arg->l3cc = table;
+
+ if (has_mocs(gt->i915))
+ arg->mocs = table;
+
arg->scratch = create_scratch(gt);
if (IS_ERR(arg->scratch))
return PTR_ERR(arg->scratch);
@@ -223,9 +243,9 @@ static int check_mocs_engine(struct live_mocs *arg,
/* Read the mocs tables back using SRM */
offset = i915_ggtt_offset(vma);
if (!err)
- err = read_mocs_table(rq, &arg->table, &offset);
+ err = read_mocs_table(rq, &arg->mocs, &offset);
if (!err && ce->engine->class == RENDER_CLASS)
- err = read_l3cc_table(rq, &arg->table, &offset);
+ err = read_l3cc_table(rq, &arg->l3cc, &offset);
offset -= i915_ggtt_offset(vma);
GEM_BUG_ON(offset > PAGE_SIZE);
@@ -236,9 +256,9 @@ static int check_mocs_engine(struct live_mocs *arg,
/* Compare the results against the expected tables */
vaddr = arg->vaddr;
if (!err)
- err = check_mocs_table(ce->engine, &arg->table, &vaddr);
+ err = check_mocs_table(ce->engine, &arg->mocs, &vaddr);
if (!err && ce->engine->class == RENDER_CLASS)
- err = check_l3cc_table(ce->engine, &arg->table, &vaddr);
+ err = check_l3cc_table(ce->engine, &arg->l3cc, &vaddr);
if (err)
return err;
we could retain the table driven approach?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related
* [Bug 1863025] Re: Use-after-free after flush in TCG accelerator
From: Yifan @ 2020-02-14 18:09 UTC (permalink / raw)
To: qemu-devel
In-Reply-To: <158154486735.14935.3370403781300872079.malonedeb@soybean.canonical.com>
I found it just by launching Ubuntu 19.10 live cd with QXL driver. I
will re-test this weekend.
The workaround I had is to check the number of TLB flushes and to re-try
obtaining the TB if the number changes. There is a penalty for the case
where TLB is flushed but should not degrade performance in most cases. I
think obtaining the lock earlier will slow down the VM if EXCP_ATOMIC is
used often.
Of course, I am assuming TLB flush is the only way to cause this bug.
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index d1c2b6ea1fd..d83b578299b 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -250,8 +250,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
uint32_t flags;
uint32_t cflags = 1;
uint32_t cf_mask = cflags & CF_HASH_MASK;
+ unsigned flush_count;
if (sigsetjmp(cpu->jmp_env, 0) == 0) {
+retry:
+ flush_count = tb_flush_count();
tb = tb_lookup__cpu_state(cpu, &pc, &cs_base, &flags, cf_mask);
if (tb == NULL) {
mmap_lock();
@@ -260,6 +263,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
}
start_exclusive();
+ /* do_tb_flush() might run and make tb invalid */
+ if (flush_count != tb_flush_count()) {
+ end_exclusive();
+ goto retry;
+ }
/* Since we got here, we know that parallel_cpus must be true. */
parallel_cpus = false;
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 4ed9d0abaf2..ecf7d3b53ff 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2696,6 +2696,11 @@ void tcg_flush_softmmu_tlb(CPUState *cs)
#endif
}
+unsigned tb_flush_count(void)
+{
+ return atomic_read(&tb_ctx.tb_flush_count);
+}
+
#if defined(CONFIG_NO_RWX)
void tb_exec_memory_lock(void)
{
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 5ccc9485812..1bc61fa6d76 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -584,6 +584,7 @@ void tlb_set_dirty(CPUState *cpu, target_ulong vaddr);
void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr);
/* translate-all.c */
+unsigned tb_flush_count(void);
#if defined(CONFIG_NO_RWX)
void tb_exec_memory_lock(void);
bool tb_is_exec(const TranslationBlock *tb);
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1863025
Title:
Use-after-free after flush in TCG accelerator
Status in QEMU:
Confirmed
Bug description:
I believe I found a UAF in TCG that can lead to a guest VM escape. The
security list informed me "This can not be treated as a security
issue." and to post it here. I am looking at the 4.2.0 source code.
The issue requires a race and I will try to describe it in terms of
three concurrent threads.
Thread A:
A1. qemu_tcg_cpu_thread_fn runs work loop
A2. qemu_wait_io_event => qemu_wait_io_event_common => process_queued_cpu_work
A3. start_exclusive critical section entered
A4. do_tb_flush is called, TB memory freed/re-allocated
A5. end_exclusive exits critical section
Thread B:
B1. qemu_tcg_cpu_thread_fn runs work loop
B2. tcg_cpu_exec => cpu_exec => tb_find => tb_gen_code
B3. tcg_tb_alloc obtains a new TB
Thread C:
C1. qemu_tcg_cpu_thread_fn runs work loop
C2. cpu_exec_step_atomic executes
C3. TB obtained with tb_lookup__cpu_state or tb_gen_code
C4. start_exclusive critical section entered
C5. cpu_tb_exec executes the TB code
C6. end_exclusive exits critical section
Consider the following sequence of events:
B2 => B3 => C3 (same TB as B2) => A3 => A4 (TB freed) => A5 => B2 =>
B3 (re-allocates TB from B2) => C4 => C5 (freed/reused TB now executing) => C6
In short, because thread C uses the TB in the critical section, there
is no guarantee that the pointer has not been "freed" (rather the
memory is marked as re-usable) and therefore a use-after-free occurs.
Since the TCG generated code can be in the same memory as the TB data
structure, it is possible for an attacker to overwrite the UAF pointer
with code generated from TCG. This can overwrite key pointer values
and could lead to code execution on the host outside of the TCG
sandbox.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1863025/+subscriptions
^ permalink raw reply related
* [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK
From: Marek Vasut @ 2020-02-14 18:29 UTC (permalink / raw)
To: u-boot
In-Reply-To: <72f2e4d1e8f84cf481ec285c5101b6b8@SFHDAG6NODE3.st.com>
On 2/14/20 2:34 PM, Patrick DELAUNAY wrote:
> Hi,
[...]
>> drivers/usb/host/dwc2.c | 100
>> +++++++++++++++++++++++++++++++++++++++-
>> include/clk.h | 4 ++
>> 2 files changed, 103 insertions(+), 1 deletion(-)
>>
>> --
>> 2.17.1
>
> Any comments on this serie or I need to rebase it and resend the serie ?
>
> http://patchwork.ozlabs.org/project/uboot/list/?series=142257
>
> Sorry for the delay...
> I miss the previous merge windows for v2020.01 and now I think it also the case for v2020.04 !
Please rebase and repost, thanks.
[...]
--
Best regards,
Marek Vasut
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.