[PATCH] char/agp: Disable frontend without CONFIG_DRM_LEGACY

[Christian König]

Am 18.11.20 um 10:02 schrieb Thomas Zimmermann:
> Hi
>
> Am 18.11.20 um 09:53 schrieb Daniel Vetter:
>> On Wed, Nov 18, 2020 at 9:24 AM Christian König
>> <christian.koenig at amd.com> wrote:
>>>
>>> Am 18.11.20 um 09:02 schrieb Thomas Zimmermann:
>>>> Hi
>>>>
>>>> Am 17.11.20 um 22:40 schrieb Daniel Vetter:
>>>>> It's probably full of bugs ready for exploiting by userspace. And
>>>>> there's not going to be any userspace for this without any of the drm
>>>>> legacy drivers enabled too. So just couple it together.
>>>>>
>>>>> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>>>>> Cc: David Airlie <airlied at linux.ie>
>>>>> Cc: Adam Jackson <ajax at redhat.com>
>>>>> ---
>>>>>    drivers/char/agp/Makefile | 6 +++++-
>>>>>    drivers/char/agp/agp.h    | 5 +++++
>>>>>    2 files changed, 10 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/char/agp/Makefile b/drivers/char/agp/Makefile
>>>>> index cb2497d157f6..90ed8c789e48 100644
>>>>> --- a/drivers/char/agp/Makefile
>>>>> +++ b/drivers/char/agp/Makefile
>>>>> @@ -1,7 +1,11 @@
>>>>>    # SPDX-License-Identifier: GPL-2.0
>>>>> -agpgart-y := backend.o frontend.o generic.o isoch.o
>>>>> +agpgart-y := backend.o generic.o isoch.o
>>>>>
>>>>> +ifeq ($(CONFIG_DRM_LEGACY),y)
>>>>>    agpgart-$(CONFIG_COMPAT)   += compat_ioctl.o
>>>>> +agpgart-y                   += frontend.o
>>>>> +endif
>>>>> +
>>>>>
>>>>>    obj-$(CONFIG_AGP)          += agpgart.o
>>>>>    obj-$(CONFIG_AGP_ALI)              += ali-agp.o
>>>>> diff --git a/drivers/char/agp/agp.h b/drivers/char/agp/agp.h
>>>>> index 4eb1c772ded7..bb09d64cd51e 100644
>>>>> --- a/drivers/char/agp/agp.h
>>>>> +++ b/drivers/char/agp/agp.h
>>>>> @@ -186,8 +186,13 @@ int agp_add_bridge(struct agp_bridge_data 
>>>>> *bridge);
>>>>>    void agp_remove_bridge(struct agp_bridge_data *bridge);
>>>>>
>>>>>    /* Frontend routines. */
>>>>> +#if IS_ENABLED(CONFIG_DRM_LEGACY)
>>>>>    int agp_frontend_initialize(void);
>>>>>    void agp_frontend_cleanup(void);
>>>>> +#else
>>>>> +static inline int agp_frontend_initialize(void) { return 0; }
>>>>> +static inline void agp_frontend_cleanup(void) {}
>>>>> +#endif
>>>> There's one non-legacy driver that uses these agp structures, which is
>>>> radeon. Does this change affect radeon?
>>>
>>> Nouveau uses AGP as well, but I'm not sure if both drivers use any of
>>> this stuff.
>>
>> frontend = /dev/agp chardev interface for userspace drivers. If you're
>> looking at kernel drivers, you're looking at the wrong thing, the
>> kernel-internal interface is in char/agp/backend.c and still enabled.
>> So no impact at all on any kernel code.
>>
>> Now the impact this does have on kms drivers using agp is that there's
>> no longer a userspace ioctl interface to change the agp setup and
>> mappings and fight the kms driver (which assumes it's fully in control
>> of agp configuration).

I was strongly assuming something like this, thanks for clarifying.

>
> Thanks for clarifying. I'm certainly not qualified, but still
>
> Acked-by: Thomas Zimmermann <tzimmermann at suse.de>

Acked-by: Christian König <christian.koenig at amd.com> as well.

Regards,
Christian.

>
> Best regards
> Thomas
>
>> -Daniel
>>
>>>
>>> Regards,
>>> Christian.
>>>
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>>
>>>>>    /* Generic routines. */
>>>>>    void agp_generic_enable(struct agp_bridge_data *bridge, u32 mode);
>>>>>
>>>
>>
>>
>