| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7fc41ebc-bbea-44af-80b2-c2baee6eb1a6@kernel.org>
Date: Tue, 16 Dec 2025 08:48:38 -0600
From: Mario Limonciello <superm1@...nel.org>
To: "Lazar, Lijo" <lijo.lazar@....com>,
"Yo-Jung Leo Lin (AMD)" <Leo.Lin@....com>,
Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
Jonathan Corbet <corbet@....net>
Cc: amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
"Tsao, Anson" <anson.tsao@....com>
Subject: Re: [PATCH v6 4/5] drm/amdgpu: add UMA allocation interfaces to sysfs
On 12/16/25 8:44 AM, Lazar, Lijo wrote:
>
>
> On 16-Dec-25 7:38 PM, Mario Limonciello wrote:
>> On 12/16/25 3:43 AM, Lazar, Lijo wrote:
>>>
>>>
>>> On 12-Dec-25 1:29 PM, Yo-Jung Leo Lin (AMD) wrote:
>>>> Add a uma/ directory containing two sysfs files as interfaces to
>>>> inspect or change UMA carveout size. These files are:
>>>>
>>>> - uma/carveout_options: a read-only file listing all the available
>>>> UMA allocation options and their index.
>>>>
>>>> - uma/carveout: a file that is both readable and writable. On read,
>>>> it shows the index of the current setting. Writing a valid index
>>>> into this file allows users to change the UMA carveout size to that
>>>> option on the next boot.
>>>>
>>>> Co-developed-by: Mario Limonciello (AMD) <superm1@...nel.org>
>>>> Signed-off-by: Mario Limonciello (AMD) <superm1@...nel.org>
>>>> Reviewed-by: Alex Deucher <alexander.deucher@....com>
>>>> Signed-off-by: Yo-Jung Leo Lin (AMD) <Leo.Lin@....com>
>>>> ---
>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 143 +++++++++++++++++
>>>> + + ++++++++++
>>>> 1 file changed, 143 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/
>>>> gpu/ drm/amd/amdgpu/amdgpu_device.c
>>>> index 903c4706040d..e78e6982312c 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>>> @@ -36,6 +36,7 @@
>>>> #include <linux/pci.h>
>>>> #include <linux/pci-p2pdma.h>
>>>> #include <linux/apple-gmux.h>
>>>> +#include <linux/nospec.h>
>>>> #include <drm/drm_atomic_helper.h>
>>>> #include <drm/drm_client_event.h>
>>>> @@ -417,6 +418,146 @@ static const struct attribute_group
>>>> amdgpu_board_attrs_group = {
>>>> .is_visible = amdgpu_board_attrs_is_visible
>>>> };
>>>> +static ssize_t carveout_options_show(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + char *buf)
>>>> +{
>>>> + struct drm_device *ddev = dev_get_drvdata(dev);
>>>> + struct amdgpu_device *adev = drm_to_adev(ddev);
>>>> + struct amdgpu_uma_carveout_info *uma_info = &adev->uma_info;
>>>> + uint32_t memory_carved;
>>>> + ssize_t size = 0;
>>>> +
>>>> + if (!uma_info || !uma_info->num_entries)
>>>> + return -ENODEV;
>>>> +
>>>> + for (int i = 0; i < uma_info->num_entries; i++) {
>>>> + memory_carved = uma_info->entries[i].memory_carved_mb;
>>>> + if (memory_carved >= SZ_1G/SZ_1M) {
>>>> + size += sysfs_emit_at(buf, size, "%d: %s (%u GB)\n",
>>>> + i,
>>>> + uma_info->entries[i].name,
>>>> + memory_carved >> 10);
>>>> + } else {
>>>> + size += sysfs_emit_at(buf, size, "%d: %s (%u MB)\n",
>>>> + i,
>>>> + uma_info->entries[i].name,
>>>> + memory_carved);
>>>> + }
>>>> + }
>>>> +
>>>> + return size;
>>>> +}
>>>> +static DEVICE_ATTR_RO(carveout_options);
>>>> +
>>>> +static ssize_t carveout_show(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + char *buf)
>>>> +{
>>>> + struct drm_device *ddev = dev_get_drvdata(dev);
>>>> + struct amdgpu_device *adev = drm_to_adev(ddev);
>>>> +
>>>> + return sysfs_emit(buf, "%u\n", adev->uma_info.uma_option_index);
>>>
>>> It would be better to show the size along with the index.
>>
>> This idea complicates userspace parsing because rather than a atoi()
>> parser you now need to do sscanf().
>>
>> IE - I like it the way it is in the series at the moment.
>>
>
> Userspace already has to go through this for carveout_options parsing.
> It does not make any difference :)
Oh I guess you're right, userspace wouldn't just present a string from
carveout options, it would need to know what index went with what string
which means parsing to populate a UI (for example).
>
> I was thinking of an easier way without cross reference. It would be
> better even if you put the actual carve out size instead of an index
> (assuming carve out size is deterministic).
>
> Thanks,
> Lijo
>
>
>>>
>>> Regardless, series is -
>>>
>>> Reviewed-by: Lijo Lazar <lijo.lazar@....com>
>>>
>>> Thanks,
>>> Lijo
>>>
>>>> +}
>>>> +
>>>> +static ssize_t carveout_store(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + const char *buf, size_t count)
>>>> +{
>>>> + struct drm_device *ddev = dev_get_drvdata(dev);
>>>> + struct amdgpu_device *adev = drm_to_adev(ddev);
>>>> + struct amdgpu_uma_carveout_info *uma_info = &adev->uma_info;
>>>> + struct amdgpu_uma_carveout_option *opt;
>>>> + unsigned long val;
>>>> + uint8_t flags;
>>>> + int r;
>>>> +
>>>> + r = kstrtoul(buf, 10, &val);
>>>> + if (r)
>>>> + return r;
>>>> +
>>>> + if (val >= uma_info->num_entries)
>>>> + return -EINVAL;
>>>> +
>>>> + val = array_index_nospec(val, uma_info->num_entries);
>>>> + opt = &uma_info->entries[val];
>>>> +
>>>> + if (!(opt->flags & AMDGPU_UMA_FLAG_AUTO) &&
>>>> + !(opt->flags & AMDGPU_UMA_FLAG_CUSTOM)) {
>>>> + drm_err_once(ddev, "Option %lu not supported due to lack of
>>>> Custom/Auto flag", val);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + flags = opt->flags;
>>>> + flags &= ~((flags & AMDGPU_UMA_FLAG_AUTO) >> 1);
>>>> +
>>>> + guard(mutex)(&uma_info->update_lock);
>>>> +
>>>> + r = amdgpu_acpi_set_uma_allocation_size(adev, val, flags);
>>>> + if (r)
>>>> + return r;
>>>> +
>>>> + uma_info->uma_option_index = val;
>>>> +
>>>> + return count;
>>>> +}
>>>> +static DEVICE_ATTR_RW(carveout);
>>>> +
>>>> +static struct attribute *amdgpu_uma_attrs[] = {
>>>> + &dev_attr_carveout.attr,
>>>> + &dev_attr_carveout_options.attr,
>>>> + NULL
>>>> +};
>>>> +
>>>> +const struct attribute_group amdgpu_uma_attr_group = {
>>>> + .name = "uma",
>>>> + .attrs = amdgpu_uma_attrs
>>>> +};
>>>> +
>>>> +static void amdgpu_uma_sysfs_init(struct amdgpu_device *adev)
>>>> +{
>>>> + int rc;
>>>> +
>>>> + if (!(adev->flags & AMD_IS_APU))
>>>> + return;
>>>> +
>>>> + if (!amdgpu_acpi_is_set_uma_allocation_size_supported())
>>>> + return;
>>>> +
>>>> + rc = amdgpu_atomfirmware_get_uma_carveout_info(adev, &adev-
>>>> >uma_info);
>>>> + if (rc) {
>>>> + drm_dbg(adev_to_drm(adev),
>>>> + "Failed to parse UMA carveout info from VBIOS: %d\n", rc);
>>>> + goto out_info;
>>>> + }
>>>> +
>>>> + mutex_init(&adev->uma_info.update_lock);
>>>> +
>>>> + rc = devm_device_add_group(adev->dev, &amdgpu_uma_attr_group);
>>>> + if (rc) {
>>>> + drm_dbg(adev_to_drm(adev), "Failed to add UMA carveout
>>>> sysfs interfaces %d\n", rc);
>>>> + goto out_attr;
>>>> + }
>>>> +
>>>> + return;
>>>> +
>>>> +out_attr:
>>>> + mutex_destroy(&adev->uma_info.update_lock);
>>>> +out_info:
>>>> + return;
>>>> +}
>>>> +
>>>> +static void amdgpu_uma_sysfs_fini(struct amdgpu_device *adev)
>>>> +{
>>>> + struct amdgpu_uma_carveout_info *uma_info = &adev->uma_info;
>>>> +
>>>> + if (!amdgpu_acpi_is_set_uma_allocation_size_supported())
>>>> + return;
>>>> +
>>>> + mutex_destroy(&uma_info->update_lock);
>>>> + uma_info->num_entries = 0;
>>>> +}
>>>> +
>>>> static void amdgpu_device_get_pcie_info(struct amdgpu_device *adev);
>>>> /**
>>>> @@ -4492,6 +4633,7 @@ static int
>>>> amdgpu_device_sys_interface_init(struct amdgpu_device *adev)
>>>> amdgpu_fru_sysfs_init(adev);
>>>> amdgpu_reg_state_sysfs_init(adev);
>>>> amdgpu_xcp_sysfs_init(adev);
>>>> + amdgpu_uma_sysfs_init(adev);
>>>> return r;
>>>> }
>>>> @@ -4507,6 +4649,7 @@ static void
>>>> amdgpu_device_sys_interface_fini(struct amdgpu_device *adev)
>>>> amdgpu_reg_state_sysfs_fini(adev);
>>>> amdgpu_xcp_sysfs_fini(adev);
>>>> + amdgpu_uma_sysfs_fini(adev);
>>>> }
>>>> /**
>>>>
>>>
>>
>
Powered by blists - more mailing lists