Re: [PATCH v5 2/3] x86/mm: add API for marking only part of a MMIO page read only

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Mon, Jul 22, 2024 at 03:01:45PM +0200, Jan Beulich wrote:
> On 22.07.2024 14:36, Marek Marczykowski-Górecki wrote:
> > On Mon, Jul 22, 2024 at 02:09:15PM +0200, Jan Beulich wrote:
> >> On 19.07.2024 04:33, Marek Marczykowski-Górecki wrote:
> >>> +int __init subpage_mmio_ro_add(
> >>> +    paddr_t start,
> >>> +    size_t size)
> >>> +{
> >>> +    mfn_t mfn_start = maddr_to_mfn(start);
> >>> +    paddr_t end = start + size - 1;
> >>> +    mfn_t mfn_end = maddr_to_mfn(end);
> >>> +    unsigned int offset_end = 0;
> >>> +    int rc;
> >>> +    bool subpage_start, subpage_end;
> >>> +
> >>> +    ASSERT(IS_ALIGNED(start, MMIO_RO_SUBPAGE_GRAN));
> >>> +    ASSERT(IS_ALIGNED(size, MMIO_RO_SUBPAGE_GRAN));
> >>> +    if ( !IS_ALIGNED(size, MMIO_RO_SUBPAGE_GRAN) )
> >>> +        return -EINVAL;
> >>
> >> I think I had asked before: Why is misaligned size something that wants a
> >> release build fallback to the assertion, but not misaligned start?
> > 
> > Misaligned start will lead to protecting larger area, not smaller, so it
> > is not unsafe thing to do. But I can also make it return an error, it
> > shouldn't happen after all.
> 
> Well, I wouldn't mind if you kept what you have, just with a (brief) comment
> making clear why there is a difference in treatment. After all you could
> treat mis-aligned size similarly, making the protected area larger, too.

Ok.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature