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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

Jan