On 21.05.2024 17:33, Marek Marczykowski-Górecki wrote:
> On Tue, May 21, 2024 at 05:16:58PM +0200, Jan Beulich wrote:
>> On 21.05.2024 04:54, Marek Marczykowski-Górecki wrote:
>>> --- a/xen/arch/x86/include/asm/mm.h
>>> +++ b/xen/arch/x86/include/asm/mm.h
>>> @@ -522,9 +522,27 @@ extern struct rangeset *mmio_ro_ranges;
>>> void memguard_guard_stack(void *p);
>>> void memguard_unguard_stack(void *p);
>>>
>>> +/*
>>> + * Add more precise r/o marking for a MMIO page. Range specified here
>>> + * will still be R/O, but the rest of the page (not marked as R/O via
>>> another
>>> + * call) will have writes passed through.
>>> + * The start address and the size must be aligned to SUBPAGE_MMIO_RO_ALIGN.
>>> + *
>>> + * This API cannot be used for overlapping ranges, nor for pages already
>>> added
>>> + * to mmio_ro_ranges separately.
>>> + *
>>> + * Return values:
>>> + * - negative: error
>>> + * - 0: success
>>> + */
>>> +#define SUBPAGE_MMIO_RO_ALIGN 8
>>
>> This isn't just alignment, but also (and perhaps more importantly)
>> granularity.
>> I think the name wants to express this.
>
> SUBPAGE_MMIO_RO_GRANULARITY? Sounds a bit long...
..._GRAN? ..._CHUNK? ..._UNIT? (Perhaps also want to have MMIO_RO_ first.)
>>> +static int __init subpage_mmio_ro_add_page(
>>> + mfn_t mfn, unsigned int offset_s, unsigned int offset_e)
>>> +{
>>> + struct subpage_ro_range *entry = NULL, *iter;
>>> + unsigned int i;
>>> +
>>> + list_for_each_entry(iter, &subpage_ro_ranges, list)
>>> + {
>>> + if ( mfn_eq(iter->mfn, mfn) )
>>> + {
>>> + entry = iter;
>>> + break;
>>> + }
>>> + }
>>> + if ( !entry )
>>> + {
>>> + /* iter == NULL marks it was a newly allocated entry */
>>> + iter = NULL;
>>> + entry = xzalloc(struct subpage_ro_range);
>>> + if ( !entry )
>>> + return -ENOMEM;
>>> + entry->mfn = mfn;
>>> + }
>>> +
>>> + for ( i = offset_s; i <= offset_e; i += SUBPAGE_MMIO_RO_ALIGN )
>>> + {
>>> + int oldbit = __test_and_set_bit(i / SUBPAGE_MMIO_RO_ALIGN,
>>> + entry->ro_qwords);
>>
>> Why int, not bool?
>
> Because __test_and_set_bit returns int. But I can change to bool if you
> prefer.
test_bit() and the test-and set of functions should eventually all change
to be returning bool. One less use site to then also take care of.
Jan
