-----Original Message----- From: Brost, Matthew <[email protected]> Sent: Thursday, November 20, 2025 9:32 AM To: Cavitt, Jonathan <[email protected]> Cc: [email protected]; Gupta, saurabhg <[email protected]>; Zuo, Alex <[email protected]>; [email protected]; Zhang, Jianxun <[email protected]>; Lin, Shuicheng <[email protected]>; [email protected]; Wajdeczko, Michal <[email protected]>; Mrozek, Michal <[email protected]>; Jadav, Raag <[email protected]>; [email protected]; Briano, Ivan <[email protected]>; Auld, Matthew <[email protected]>; Hirschfeld, Dafna <[email protected]> Subject: Re: [PATCH v26 1/4] drm/xe/xe_pagefault: Disallow writes to read-only VMAs > > On Wed, Nov 19, 2025 at 07:53:24PM +0000, Jonathan Cavitt wrote: > > The page fault handler should reject write/atomic access to read only > > VMAs. Add code to handle this in xe_pagefault_service after the VMA > > lookup. > > > > Fixes: fb544b844508 ("drm/xe: Implement xe_pagefault_queue_work") > > Signed-off-by: Jonathan Cavitt <[email protected]> > > Suggested-by: Matthew Brost <[email protected]> > > Cc: Shuicheng Lin <[email protected]> > > --- > > drivers/gpu/drm/xe/xe_pagefault.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/xe/xe_pagefault.c > > b/drivers/gpu/drm/xe/xe_pagefault.c > > index fe3e40145012..836c39010f02 100644 > > --- a/drivers/gpu/drm/xe/xe_pagefault.c > > +++ b/drivers/gpu/drm/xe/xe_pagefault.c > > @@ -188,6 +188,11 @@ static int xe_pagefault_service(struct xe_pagefault > > *pf) > > goto unlock_vm; > > } > > > > + if (xe_vma_read_only(vma) && pf->consumer.access_type != > > XE_PAGEFAULT_ACCESS_TYPE_READ) { > > Nit: Prefer 80 characters per line if possible and doesn't made the code > super ugly. That really is a personal preference but all code I author > (this file) tries to do this, so prefer to keep that way. > > Either way: > Reviewed-by: Matthew Brost <[email protected]>
Thank you for your review! Applying the requested NIT should be simple enough, though I plan on waiting for more revision notes on the rest of the series before applying this change. Thank you for your patience. -Jonathan Cavitt > > > + err = -EPERM; > > + goto unlock_vm; > > + } > > + > > atomic = xe_pagefault_access_is_atomic(pf->consumer.access_type); > > > > if (xe_vma_is_cpu_addr_mirror(vma)) > > -- > > 2.43.0 > > >
