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

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

On Thu, Jul 25, 2024 at 11:26:31AM +0200, Jan Beulich wrote:
> On 23.07.2024 05:24, Marek Marczykowski-Górecki wrote:
> > +     * so tolerate it.
> > +     * But unaligned size would result in smaller area, so deny it.
> > +     */
> > +    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 hoped you would, when adding the comment, recall an earlier comment of
> mine: If you want to tolerate mis-aligned start in release builds, you
> need to make further adjustments to the subsequent logic (at which
> point the respective assertion may become pointless); see below. While
> things may work okay without (I didn't fully convince myself either way),
> the main point here is that you want to make sure we test in debug builds
> what's actually used in release one. Hence subtleties like this would
> better be dealt with uniformly between release and debug builds.

Right, and I think this is a good argument to not try to accept
unaligned size either, even if it would be possible here.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]