RE: [PATCH 1/2] x86: Add Isolated Memory Regions for Quark X1000

[Ong, Boon Leong]

>>> +/**
>>> + * imr_del_range - Delete an Isolated Memory Region
>>> + * @reg: IMR index to remove
>>> + * @base: Physical base address of region aligned to 4k
>>> + * @size: Physical size of region in bytes
>>> + * @return:	-EINVAL on invalid range or out or range id
>>> + *		-ENODEV if reg is valid but no IMR exists or is locked
>>> + *		0 on success
>>> + */
>>> +int imr_del(int reg, unsigned long base, unsigned long size);
>>
>> How about just offer imr delete based index-only as returned by imr_add()?
>> We just need to check if that IMR is locked. If locked, then bail out.
>> Else, we will zero-out IMR register for that index to remove it.
>
>Hmm.
>
>The MTRR API this is based on allows you to specific an address range and I think
>that makes sense for an IMR API too because - say you want to tear down the
>IMR around the kernel .text area - but you don't know which IMR it is.
>
>You'd just do
>
>unsigned long base  = virt_to_phys(&_text); unsigned long size =
>virt_to_phys(&_sinittext) - base - IMR_ALIGN;
>
>imr_del(-1, base, size);
>
>Rather than having to know which specific index to kill. Also later silicon may
>have more - or less IMR indices - so deleting based on an address range is a
>valuable feature I think.

imr_add() returns IMR index, so I would expect that I can use the index directly
if I need to remove it. 

Suggest to split the imr_del() into 2 functions:- 
(1) by address + size 
(2) by IMR index
At current implementation, it does not support (2) only because it fails at 
imr_check_range().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/