Re: [Nouveau] [PATCH v2 0/8] Fix several device private page reference counting issues

2022-10-25 Thread Alistair Popple


"Vlastimil Babka (SUSE)"  writes:

> On 9/28/22 14:01, Alistair Popple wrote:
>> This series aims to fix a number of page reference counting issues in
>> drivers dealing with device private ZONE_DEVICE pages. These result in
>> use-after-free type bugs, either from accessing a struct page which no
>> longer exists because it has been removed or accessing fields within the
>> struct page which are no longer valid because the page has been freed.
>>
>> During normal usage it is unlikely these will cause any problems. However
>> without these fixes it is possible to crash the kernel from userspace.
>> These crashes can be triggered either by unloading the kernel module or
>> unbinding the device from the driver prior to a userspace task exiting. In
>> modules such as Nouveau it is also possible to trigger some of these issues
>> by explicitly closing the device file-descriptor prior to the task exiting
>> and then accessing device private memory.
>
> Hi, as this series was noticed to create a CVE [1], do you think a stable
> backport is warranted? I think the "It is possible to launch the attack
> remotely." in [1] is incorrect though, right?

Right, I don't see how this could be exploited remotely. And I'm pretty
sure you need root as well because in practice the pgmap needs to be
freed, and for Nouveau at least that only happens on device removal.

> It looks to me that patch 1 would be needed since the CONFIG_DEVICE_PRIVATE
> introduction, while the following few only to kernels with 27674ef6c73f
> (probably not so critical as that includes no LTS)?

Patch 3 already has a fixes tag for 27674ef6c73f. Patch 1 would need to
go back to CONFIG_DEVICE_PRIVATE introduction. I think patches 4-8 would
also need to go back to introduction of CONFIG_DEVICE_PRIVATE, but there
isn't as much impact there and they would be harder to backport I think.
Without them device removal can loop indefinitely in kernel mode (if
patch 3 is present or the kernel is older than 27674ef6c73f).

 - Alistair

> Thanks,
> Vlastimil
>
> [1] https://nvd.nist.gov/vuln/detail/CVE-2022-3523
>
>> This involves some minor changes to both PowerPC and AMD GPU code.
>> Unfortunately I lack hardware to test either of those so any help there
>> would be appreciated. The changes mimic what is done in for both Nouveau
>> and hmm-tests though so I doubt they will cause problems.
>>
>> To: Andrew Morton 
>> To: linux...@kvack.org
>> Cc: linux-ker...@vger.kernel.org
>> Cc: amd-...@lists.freedesktop.org
>> Cc: nouveau@lists.freedesktop.org
>> Cc: dri-de...@lists.freedesktop.org
>>
>> Alistair Popple (8):
>>   mm/memory.c: Fix race when faulting a device private page
>>   mm: Free device private pages have zero refcount
>>   mm/memremap.c: Take a pgmap reference on page allocation
>>   mm/migrate_device.c: Refactor migrate_vma and 
>> migrate_deivce_coherent_page()
>>   mm/migrate_device.c: Add migrate_device_range()
>>   nouveau/dmem: Refactor nouveau_dmem_fault_copy_one()
>>   nouveau/dmem: Evict device private memory during release
>>   hmm-tests: Add test for migrate_device_range()
>>
>>  arch/powerpc/kvm/book3s_hv_uvmem.c   |  17 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.c |  19 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_migrate.h |   2 +-
>>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c |  11 +-
>>  drivers/gpu/drm/nouveau/nouveau_dmem.c   | 108 +++
>>  include/linux/memremap.h |   1 +-
>>  include/linux/migrate.h  |  15 ++-
>>  lib/test_hmm.c   | 129 ++---
>>  lib/test_hmm_uapi.h  |   1 +-
>>  mm/memory.c  |  16 +-
>>  mm/memremap.c|  30 ++-
>>  mm/migrate.c |  34 +--
>>  mm/migrate_device.c  | 239 +---
>>  mm/page_alloc.c  |   8 +-
>>  tools/testing/selftests/vm/hmm-tests.c   |  49 +-
>>  15 files changed, 516 insertions(+), 163 deletions(-)
>>
>> base-commit: 088b8aa537c2c767765f1c19b555f21ffe555786


Re: [Nouveau] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

2022-10-25 Thread Matthew Garrett
On Wed, Oct 26, 2022 at 01:27:25AM +0200, Hans de Goede wrote:

> this code should actually set the ACPI_VIDEO_BACKLIGHT flag:
> drivers/acpi/scan.c:
> 
> static acpi_status
> acpi_backlight_cap_match(acpi_handle handle, u32 level, void *context,
>   void **return_value)
> {
> long *cap = context;
> 
> if (acpi_has_method(handle, "_BCM") &&
> acpi_has_method(handle, "_BCL")) {
> acpi_handle_debug(handle, "Found generic backlight 
> support\n");
> *cap |= ACPI_VIDEO_BACKLIGHT;
> /* We have backlight support, no need to scan further */
> return AE_CTRL_TERMINATE;
> }
> return 0;
> }

Ah, yeah, my local tree no longer matches the upstream behaviour because 
I've hacked the EC firmware to remove the backlight trigger because it 
had an extremely poor brightness curve and also automatically changed it 
on AC events - as a result I removed the backlight code from the DSDT 
and just fell back to the native control. Like I said I'm a long way 
from the normal setup, but this did previously work.

The "right" logic here seems pretty simple: if ACPI backlight control is 
expected to work, use it. If it isn't, but there's a vendor interface, 
use it. If there's no vendor interface, use the native interface. The 
problem you're dealing with is that the knowledge of whether or not 
there's a vendor interface isn't something the core kernel code knows 
about. What you're proposing here is effectively for us to expose 
additional information about whether or not there's a vendor interface 
in the system firmware, but since we're talking in some cases about 
hardware that's almost 20 years old, we're not realistically going to 
get those old machines fixed. So, it feels like there's two choices:

1) Make a default policy decision, but then allow that decision to be 
altered later on (eg, when a vendor-specific platform driver has been 
loaded) - you've said this poses additional complexities.

2) Move the knowledge of whether or not there's a vendor interface into 
the core code. Basically take every platform driver that exposes a 
vendor interface, and move the detection code into the core.

I think any other approach is going to result in machines that 
previously worked no longer working (and you can't just make the 
vendor/native split dependent on the Coreboot DMI BIOS string, because 
there are some Coreboot platforms that implement the vendor interface 
for compatibility, and you also can't ask all Coreboot users to update 
their firmware to fix things)


Re: [Nouveau] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

2022-10-25 Thread Hans de Goede
Hi,

On 10/25/22 22:40, Matthew Garrett wrote:
> On Tue, Oct 25, 2022 at 10:25:33PM +0200, Hans de Goede wrote:
> 
>> Having the native driver come and then go and be replaced
>> with the vendor driver would also be quite inconvenient
>> for these planned changes.
> 
> I understand that it would be inconvenient, but you've broken existing 
> working setups.

I fully acknowledge that I have broken existing working setups
and I definitely want to see this fixed before say 6.1-rc6!

I'm not convinced (at all) that any solutions which re-introduce
acpi_video_get_backlight_type() return-s value changing
half way the boot, with some backlight interface getting
registered and then unregistered again later because
it turns out to be the wrong one is a good fix here.

The whole goal of the refactor was to leave these sorts
of shenanigans behind us.

>> Can you perhaps explain a bit in what way your laptop
>> is weird ?
> 
> It's a Chinese replacement motherboard for a Thinkpad X201, running my 
> own port of Coreboot. Its DMI strings look like an actual Thinkpad in 
> order to ensure that thinkpad_acpi can bind for hotkey suport, so it's 
> hard to quirk. It'll actually be fixed by your proposed patch to fall 
> back to native rather than vendor, but that patch will break any older 
> machines that offer a vendor interface and don't have the native control 
> hooked up (pretty sure at least the Thinkpad X40 falls into that 
> category).

So looking at:

https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src/mainboard/51nb/x210/acpi/graphics.asl

this code should actually set the ACPI_VIDEO_BACKLIGHT flag:
drivers/acpi/scan.c:

static acpi_status
acpi_backlight_cap_match(acpi_handle handle, u32 level, void *context,
  void **return_value)
{
long *cap = context;

if (acpi_has_method(handle, "_BCM") &&
acpi_has_method(handle, "_BCL")) {
acpi_handle_debug(handle, "Found generic backlight support\n");
*cap |= ACPI_VIDEO_BACKLIGHT;
/* We have backlight support, no need to scan further */
return AE_CTRL_TERMINATE;
}
return 0;
}

What does seem to be missing compared to a "normal" DSDT
is a call to _OSI("Windows 2012") so I would expect this code
in acpi_video_get_backlight_type():

/* On systems with ACPI video use either native or ACPI video. */
if (video_caps & ACPI_VIDEO_BACKLIGHT) {
/*
 * Windows 8 and newer no longer use the ACPI video interface,
 * so it often does not work. If the ACPI tables are written
 * for win8 and native brightness ctl is available, use that.
 *
 * The native check deliberately is inside the if acpi-video
 * block on older devices without acpi-video support native
 * is usually not the best choice.
 */
if (acpi_osi_is_win8() && native_available)
return acpi_backlight_native;
else
return acpi_backlight_video;
}

To enter the "return acpi_backlight_video" path since acpi_osi_is_win8()
will return false.

And then the ACPI backlight methods from:
https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src/mainboard/51nb/x210/acpi/graphics.asl

should get called when changing the backlight brightness,
so assuming that those methods work then things should work fine.

What does "ls /sys/class/backlight" output on the X210 / NB51 board
with a 6.0 kernel? And what does it output with the 6.1-rc? kernels?

IOW which backlight device / control method is being selected
and which one do you want / which one(s) do actually work?

I have been thinking about maybe doing something with 
a dmi_get_bios_year() check (see below), but that will cause
native to get prefered over vendor on old ThinkPads with
coreboot (and thus a new enough year in DMI_BIOS_DATE), which
will likely break backlight control there (if i915 offers
backlight control on those that is).

Also I wonder if it would be possible to set DMI_BIOS_VENDOR
to "Coreboot" so that we can use that? Note that thinkpad_acpi
does not care about the DMI_BIOS_VENDOR value, at least
not on models which start their DMI_PRODUCT_VERSION with
either "ThinkPad" or "Lenovo".

###

Looking more at this I notice that coreboot has a
drivers_intel_gma_displays_ssdt_generate() which seems to
at least always generate ACPI video bus ASL including
backlight control bits.

So the only reason why the current heurstics are not
returning native is the acpi_osi_is_win8() check.

So maybe that beeds to become:

if ((acpi_osi_is_win8() || dmi_get_bios_year() >= 2018) && 
native_available)
return acpi_backlight_native;
else
return acpi_backlight_video;

Although I think that will result 

Re: [Nouveau] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

2022-10-25 Thread Matthew Garrett
On Tue, Oct 25, 2022 at 10:25:33PM +0200, Hans de Goede wrote:

> Having the native driver come and then go and be replaced
> with the vendor driver would also be quite inconvenient
> for these planned changes.

I understand that it would be inconvenient, but you've broken existing 
working setups.
 
> Can you perhaps explain a bit in what way your laptop
> is weird ?

It's a Chinese replacement motherboard for a Thinkpad X201, running my 
own port of Coreboot. Its DMI strings look like an actual Thinkpad in 
order to ensure that thinkpad_acpi can bind for hotkey suport, so it's 
hard to quirk. It'll actually be fixed by your proposed patch to fall 
back to native rather than vendor, but that patch will break any older 
machines that offer a vendor interface and don't have the native control 
hooked up (pretty sure at least the Thinkpad X40 falls into that 
category).


Re: [Nouveau] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

2022-10-25 Thread Hans de Goede
Hi (again),

On 10/25/22 22:25, Hans de Goede wrote:
> Hi Matthew,
> 
> On 10/25/22 21:32, Matthew Garrett wrote:
>> On Tue, Oct 25, 2022 at 08:50:54PM +0200, Hans de Goede wrote:
>>
>>> That is a valid point, but keep in mind that this is only used on ACPI
>>> platforms and then only on devices with a builtin LCD panel and then
>>> only by GPU drivers which actually call acpi_video_get_backlight_type(),
>>> so e.g. not by all the ARM specific display drivers.
>>>
>>> So I believe that Chromebooks quite likely are the only devices with
>>> this issue.
>>
>> My laptop is, uh, weird, but it falls into this category.
>>  
 I think for this to work correctly you need to have 
 the infrastructure be aware of whether or not a vendor interface exists, 
 which means having to handle cleanup if a vendor-specific module gets 
 loaded later.
>>>
>>> Getting rid of the whole ping-ponging of which backlight drivers
>>> get loaded during boot was actually one of the goals of the rework
>>> which landed in 6.1 this actually allowed us to remove some quirks
>>> because some hw/firmware did not like us changing our mind and
>>> switching backlight interfaces after first poking another one.
>>> So we definitely don't want to go back to the ping-pong thing.
>>
>> Defaulting to native but then having a vendor driver be able to disable 
>> native drivers seems easiest? It shouldn't be a regression over the 
>> previous state of affairs since both drivers were being loaded already.
> 
> Part of the reason for the ACPI backlight detect refactor is
> because of a planned new backlight uAPI where the brightness
> control becomes a property on the drm connector object, for a
> RFC including the rationale behind this planned uAPI change see:
> https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86...@redhat.com/
> 
> These plans require that there is only 1 backlight device
> registered (per panel).
> 
> Having the native driver come and then go and be replaced
> with the vendor driver would also be quite inconvenient
> for these planned changes.
> 
> As such I would rather find a solution for your "weird"
> laptop so that acpi_video_get_backlight_type() just always
> returns vendor there.

I just realized that your have vendor driver unregister
the native one is suggested as an alternative for
the new behavior where the i915 driver no longer
registers its native backlight in cases where
acpi_video_get_backlight_type() does not return native,
and that you probably actually want the native backlight
device, right ?

So the above should read:

"so that acpi_video_get_backlight_type() just always
returns native there."

> Note that drivers/acpi/video_detect.c already has a DMI
> quirk tables for models where the heuristics from
> acpi_video_get_backlight_type() don't work. In general
> we (mostly me) try to make it so that the heuristics
> work on most models, to avoid needing to add every model
> under the sun to the DMI quirk table, but if your laptop
> is somehow special then adding a DMI quirk for it should
> be fine ?
> 
> Can you perhaps explain a bit in what way your laptop
> is weird ?

I guess it is weird in that it does not have the ACPI video,
or at least does not offer ACPI video bus backlight control
in its ACPI tables?

Can you perhaps email me an acpidump of the laptop ?

> Note that technically if the native backlight does not work,
> then the GPU driver really should not even try to register
> it. But sometimes the video-bios-tables claim the backlight
> pwm input is attached to the GPU while it is not and things
> have evolved in such a way that the DMI quirks for that
> live in acpi/video_detect.c rather then in the GPU driver.

And this bit can be ignored then because it certainly
is not relevant if you actually want the native driver.

Regards,

Hans




Re: [Nouveau] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

2022-10-25 Thread Limonciello, Mario

On 10/25/2022 15:25, Hans de Goede wrote:

Hi Matthew,

On 10/25/22 21:32, Matthew Garrett wrote:

On Tue, Oct 25, 2022 at 08:50:54PM +0200, Hans de Goede wrote:


That is a valid point, but keep in mind that this is only used on ACPI
platforms and then only on devices with a builtin LCD panel and then
only by GPU drivers which actually call acpi_video_get_backlight_type(),
so e.g. not by all the ARM specific display drivers.

So I believe that Chromebooks quite likely are the only devices with
this issue.


My laptop is, uh, weird, but it falls into this category.
  

I think for this to work correctly you need to have
the infrastructure be aware of whether or not a vendor interface exists,
which means having to handle cleanup if a vendor-specific module gets
loaded later.


Getting rid of the whole ping-ponging of which backlight drivers
get loaded during boot was actually one of the goals of the rework
which landed in 6.1 this actually allowed us to remove some quirks
because some hw/firmware did not like us changing our mind and
switching backlight interfaces after first poking another one.
So we definitely don't want to go back to the ping-pong thing.


Defaulting to native but then having a vendor driver be able to disable
native drivers seems easiest? It shouldn't be a regression over the
previous state of affairs since both drivers were being loaded already.


Part of the reason for the ACPI backlight detect refactor is
because of a planned new backlight uAPI where the brightness
control becomes a property on the drm connector object, for a
RFC including the rationale behind this planned uAPI change see:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2Fb61d3eeb-6213-afac-2e70-7b9791c86d2e%40redhat.com%2Fdata=05%7C01%7Cmario.limonciello%40amd.com%7C6380e44c87e447eedc3f08dab6c7180a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638023263541559113%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=K4oMmVl51kT9V%2B4GAdx%2FS7tXWHPKyFe5WXZzC3CPeOU%3Dreserved=0

These plans require that there is only 1 backlight device
registered (per panel).

Having the native driver come and then go and be replaced
with the vendor driver would also be quite inconvenient
for these planned changes.

As such I would rather find a solution for your "weird"
laptop so that acpi_video_get_backlight_type() just always
returns vendor there.


What exactly is this "weird" laptop?  Is it something running coreboot 
that doesn't "normally" ship with coreboot perhaps?


If that's the category it falls in, I think what we really need is 
something to land in coreboot source for indicating how it should behave 
and then also a change in the kernel to react to that.


That's a similar approach to what was used fore the chromebook laptops 
that run coreboot.




Note that drivers/acpi/video_detect.c already has a DMI
quirk tables for models where the heuristics from
acpi_video_get_backlight_type() don't work. In general
we (mostly me) try to make it so that the heuristics
work on most models, to avoid needing to add every model
under the sun to the DMI quirk table, but if your laptop
is somehow special then adding a DMI quirk for it should
be fine ?

Can you perhaps explain a bit in what way your laptop
is weird ?

Note that technically if the native backlight does not work,
then the GPU driver really should not even try to register
it. But sometimes the video-bios-tables claim the backlight
pwm input is attached to the GPU while it is not and things
have evolved in such a way that the DMI quirks for that
live in acpi/video_detect.c rather then in the GPU driver.

Regards,

Hans





Re: [Nouveau] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

2022-10-25 Thread Hans de Goede
Hi Matthew,

On 10/25/22 21:32, Matthew Garrett wrote:
> On Tue, Oct 25, 2022 at 08:50:54PM +0200, Hans de Goede wrote:
> 
>> That is a valid point, but keep in mind that this is only used on ACPI
>> platforms and then only on devices with a builtin LCD panel and then
>> only by GPU drivers which actually call acpi_video_get_backlight_type(),
>> so e.g. not by all the ARM specific display drivers.
>>
>> So I believe that Chromebooks quite likely are the only devices with
>> this issue.
> 
> My laptop is, uh, weird, but it falls into this category.
>  
>>> I think for this to work correctly you need to have 
>>> the infrastructure be aware of whether or not a vendor interface exists, 
>>> which means having to handle cleanup if a vendor-specific module gets 
>>> loaded later.
>>
>> Getting rid of the whole ping-ponging of which backlight drivers
>> get loaded during boot was actually one of the goals of the rework
>> which landed in 6.1 this actually allowed us to remove some quirks
>> because some hw/firmware did not like us changing our mind and
>> switching backlight interfaces after first poking another one.
>> So we definitely don't want to go back to the ping-pong thing.
> 
> Defaulting to native but then having a vendor driver be able to disable 
> native drivers seems easiest? It shouldn't be a regression over the 
> previous state of affairs since both drivers were being loaded already.

Part of the reason for the ACPI backlight detect refactor is
because of a planned new backlight uAPI where the brightness
control becomes a property on the drm connector object, for a
RFC including the rationale behind this planned uAPI change see:
https://lore.kernel.org/dri-devel/b61d3eeb-6213-afac-2e70-7b9791c86...@redhat.com/

These plans require that there is only 1 backlight device
registered (per panel).

Having the native driver come and then go and be replaced
with the vendor driver would also be quite inconvenient
for these planned changes.

As such I would rather find a solution for your "weird"
laptop so that acpi_video_get_backlight_type() just always
returns vendor there.

Note that drivers/acpi/video_detect.c already has a DMI
quirk tables for models where the heuristics from
acpi_video_get_backlight_type() don't work. In general
we (mostly me) try to make it so that the heuristics
work on most models, to avoid needing to add every model
under the sun to the DMI quirk table, but if your laptop
is somehow special then adding a DMI quirk for it should
be fine ?

Can you perhaps explain a bit in what way your laptop
is weird ?

Note that technically if the native backlight does not work,
then the GPU driver really should not even try to register
it. But sometimes the video-bios-tables claim the backlight
pwm input is attached to the GPU while it is not and things
have evolved in such a way that the DMI quirks for that
live in acpi/video_detect.c rather then in the GPU driver.

Regards,

Hans



Re: [Nouveau] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

2022-10-25 Thread Matthew Garrett
On Tue, Oct 25, 2022 at 08:50:54PM +0200, Hans de Goede wrote:

> That is a valid point, but keep in mind that this is only used on ACPI
> platforms and then only on devices with a builtin LCD panel and then
> only by GPU drivers which actually call acpi_video_get_backlight_type(),
> so e.g. not by all the ARM specific display drivers.
> 
> So I believe that Chromebooks quite likely are the only devices with
> this issue.

My laptop is, uh, weird, but it falls into this category.
 
> > I think for this to work correctly you need to have 
> > the infrastructure be aware of whether or not a vendor interface exists, 
> > which means having to handle cleanup if a vendor-specific module gets 
> > loaded later.
> 
> Getting rid of the whole ping-ponging of which backlight drivers
> get loaded during boot was actually one of the goals of the rework
> which landed in 6.1 this actually allowed us to remove some quirks
> because some hw/firmware did not like us changing our mind and
> switching backlight interfaces after first poking another one.
> So we definitely don't want to go back to the ping-pong thing.

Defaulting to native but then having a vendor driver be able to disable 
native drivers seems easiest? It shouldn't be a regression over the 
previous state of affairs since both drivers were being loaded already.


Re: [Nouveau] [PATCH v5 02/31] drm/i915: Don't register backlight when another backlight should be used (v2)

2022-10-25 Thread Hans de Goede
Hi,

On 10/24/22 22:30, Matthew Garrett wrote:
> On Tue, Sep 27, 2022 at 01:04:52PM +0200, Hans de Goede wrote:
> 
>> So to fix this we need to make acpi_video_get_backlight_type()
>> return native on the Acer Chromebook Spin 713.
> 
> Isn't the issue broader than that? Unless the platform is Windows 8 or 
> later, we'll *always* (outside of some corner cases) return 
> acpi_backlight_vendor if there's no ACPI video interface. This is broken 
> for any platform that implements ACPI but relies on native video 
> control, which is going to include a range of Coreboot platforms, not 
> just Chromebooks.

That is a valid point, but keep in mind that this is only used on ACPI
platforms and then only on devices with a builtin LCD panel and then
only by GPU drivers which actually call acpi_video_get_backlight_type(),
so e.g. not by all the ARM specific display drivers.

So I believe that Chromebooks quite likely are the only devices with
this issue.

We could do something like the patch which I have pasted at the end
of this email, but as its commit message notes there is a real
good chance this will cause regressions on some laptops.

So if we ever decide to go with something like the patch below,
I think we should at a minimum wait for the next cycle with that,
because 6.1 already significantly reworks the ACPI backlight
detect handling and I don't want to throw this into the mix
on top of those changes.

> I think for this to work correctly you need to have 
> the infrastructure be aware of whether or not a vendor interface exists, 
> which means having to handle cleanup if a vendor-specific module gets 
> loaded later.

Getting rid of the whole ping-ponging of which backlight drivers
get loaded during boot was actually one of the goals of the rework
which landed in 6.1 this actually allowed us to remove some quirks
because some hw/firmware did not like us changing our mind and
switching backlight interfaces after first poking another one.
So we definitely don't want to go back to the ping-pong thing.

Regards,

Hans



>From 67ee5d7163e33e65dca06887befd0639b0345883 Mon Sep 17 00:00:00 2001
From: Hans de Goede 
Date: Tue, 25 Oct 2022 20:38:56 +0200
Subject: [PATCH] ACPI: video: Simplify __acpi_video_get_backlight_type()

Simplify __acpi_video_get_backlight_type() removing a nested if which
makes the flow harder to follow.

Note this will cause a behavior change on devices which do not have
ACPI video support but do have both a vendor and native backlight
driver available. This change will cause these devices to switch
from vendor to native.

This may not be desirable in all cases, this is likely to happen
on significantly older laptops, where there very well might be
cases where the native driver does not work because the backlight is
controlled by the EC.

This removes the need for the special handling of Chromebooks,
these will now hit the if (native_available) return acpi_backlight_native;
path.

Signed-off-by: Hans de Goede 
---
 drivers/acpi/video_detect.c | 36 +++-
 1 file changed, 11 insertions(+), 25 deletions(-)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 9cd8797d12bb..9bd85b159e02 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -668,11 +668,6 @@ static const struct dmi_system_id video_detect_dmi_table[] 
= {
{ },
 };
 
-static bool google_cros_ec_present(void)
-{
-   return acpi_dev_found("GOOG0004");
-}
-
 /*
  * Determine which type of backlight interface to use on this system,
  * First check cmdline, then dmi quirks, then do autodetect.
@@ -718,30 +713,21 @@ static enum acpi_backlight_type 
__acpi_video_get_backlight_type(bool native)
if (apple_gmux_present())
return acpi_backlight_apple_gmux;
 
-   /* On systems with ACPI video use either native or ACPI video. */
-   if (video_caps & ACPI_VIDEO_BACKLIGHT) {
-   /*
-* Windows 8 and newer no longer use the ACPI video interface,
-* so it often does not work. If the ACPI tables are written
-* for win8 and native brightness ctl is available, use that.
-*
-* The native check deliberately is inside the if acpi-video
-* block on older devices without acpi-video support native
-* is usually not the best choice.
-*/
-   if (acpi_osi_is_win8() && native_available)
-   return acpi_backlight_native;
-   else
-   return acpi_backlight_video;
-   }
-
/*
-* Chromebooks that don't have backlight handle in ACPI table
-* are supposed to use native backlight if it's available.
+* Pre Windows 8, Windows uses ACPI video, so prefer that over native
+* on pre-win8 systems (Windows 8+ no longer uses ACPI video).
 */
-   if (google_cros_ec_present() && native_available)
+   

Re: [Nouveau] [PATCH v2] drm/ttm: rework on ttm_resource to use size_t type

2022-10-25 Thread Christian König

Am 25.10.22 um 17:50 schrieb Somalapuram Amaranath:

Change ttm_resource structure from num_pages to size_t size in bytes.
v1 -> v2: change PFN_UP(dst_mem->size) to ttm->num_pages
v1 -> v2: change bo->resource->size to bo->base.size at some places
v1 -> v2: remove the local variable
v1 -> v2: cleanup cmp_size_smaller_first()


Of hand that looks good to me now.

It would be nice if we keep the separation of one patch for each driver. 
But that would mean we need something like adding the size field first, 
patch all drivers and then remove num_pages which isn't a good approach 
either.


But please make sure that the Intel CI systems are happy with that.


Signed-off-by: Somalapuram Amaranath 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c|  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  3 ++-
  drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h |  4 ++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h  |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  6 +++---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c   |  8 
  drivers/gpu/drm/i915/gem/i915_gem_ttm.c|  2 +-
  drivers/gpu/drm/i915/i915_scatterlist.c|  4 ++--
  drivers/gpu/drm/i915/i915_ttm_buddy_manager.c  | 12 ++--
  drivers/gpu/drm/i915/intel_region_ttm.c|  2 +-
  drivers/gpu/drm/nouveau/nouveau_bo.c   |  4 ++--
  drivers/gpu/drm/nouveau/nouveau_bo0039.c   |  4 ++--
  drivers/gpu/drm/nouveau/nouveau_bo5039.c   |  2 +-
  drivers/gpu/drm/nouveau/nouveau_bo74c1.c   |  2 +-
  drivers/gpu/drm/nouveau/nouveau_bo85b5.c   |  4 ++--
  drivers/gpu/drm/nouveau/nouveau_bo9039.c   |  4 ++--
  drivers/gpu/drm/nouveau/nouveau_bo90b5.c   |  4 ++--
  drivers/gpu/drm/nouveau/nouveau_boa0b5.c   |  2 +-
  drivers/gpu/drm/nouveau/nouveau_gem.c  |  5 ++---
  drivers/gpu/drm/nouveau/nouveau_mem.c  |  4 ++--
  drivers/gpu/drm/nouveau/nouveau_ttm.c  |  2 +-
  drivers/gpu/drm/radeon/radeon_cs.c |  7 +--
  drivers/gpu/drm/radeon/radeon_object.c |  4 ++--
  drivers/gpu/drm/radeon/radeon_trace.h  |  2 +-
  drivers/gpu/drm/radeon/radeon_ttm.c|  4 ++--
  drivers/gpu/drm/ttm/ttm_bo.c   |  3 ---
  drivers/gpu/drm/ttm/ttm_bo_util.c  |  6 +++---
  drivers/gpu/drm/ttm/ttm_bo_vm.c|  4 ++--
  drivers/gpu/drm/ttm/ttm_range_manager.c|  2 +-
  drivers/gpu/drm/ttm/ttm_resource.c | 14 ++
  drivers/gpu/drm/vmwgfx/vmwgfx_blit.c   |  4 ++--
  drivers/gpu/drm/vmwgfx/vmwgfx_bo.c |  6 +++---
  drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c|  2 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c|  2 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c  |  6 +++---
  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c|  2 +-
  drivers/gpu/drm/vmwgfx/vmwgfx_page_dirty.c |  6 +++---
  include/drm/ttm/ttm_resource.h |  4 ++--
  38 files changed, 79 insertions(+), 81 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 1f3302aebeff..44367f03316f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -144,7 +144,7 @@ static int amdgpu_gtt_mgr_new(struct ttm_resource_manager 
*man,
node->base.start = node->mm_nodes[0].start;
} else {
node->mm_nodes[0].start = 0;
-   node->mm_nodes[0].size = node->base.num_pages;
+   node->mm_nodes[0].size = PFN_UP(node->base.size);
node->base.start = AMDGPU_BO_INVALID_OFFSET;
}
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index 2e8f6cd7a729..974e85d8b6cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -542,6 +542,7 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
/* GWS and OA don't need any alignment. */
page_align = bp->byte_align;
size <<= PAGE_SHIFT;
+
} else if (bp->domain & AMDGPU_GEM_DOMAIN_GDS) {
/* Both size and alignment must be a multiple of 4. */
page_align = ALIGN(bp->byte_align, 4);
@@ -776,7 +777,7 @@ int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr)
return 0;
}
  
-	r = ttm_bo_kmap(>tbo, 0, bo->tbo.resource->num_pages, >kmap);

+   r = ttm_bo_kmap(>tbo, 0, PFN_UP(bo->tbo.base.size), >kmap);
if (r)
return r;
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h

index 6546552e596c..5c4f93ee0c57 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_res_cursor.h
@@ -62,7 +62,7 @@ static inline void amdgpu_res_first(struct ttm_resource *res,
if (!res)