On 23.03.2018 14:46, Thomas Huth wrote:
> On 20.03.2018 12:19, Janosch Frank wrote:
>> Collaborative Memory Management is the extended vm memory usage
>> hinting for the hypervisor and handling the ESSA instruction is quite
>> complicated. Let's add at least a bare-bones test, maybe someone will
>> extend it later.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxxxxxxx>
[...]
>> +static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2)));
>> +const unsigned long page0 = (unsigned long)pagebuf;
>> +
>> +static unsigned long essa(uint8_t state, unsigned long paddr)
>> +{
>> + register uint64_t addr asm("0") = paddr;
>> + register uint64_t extr_state asm("1");
>> +
>> + asm volatile(".insn rrf,0xb9ab0000,%[extr_state],%[addr],%[new_state],0"
>> + : [extr_state] "=&d" (extr_state)
>
> Why do you need the "&" modifier here?
I took arch/s390/mm/page-states.c as a template, but after reading the
section about essa, it's not immediately clear to me why this should
have an early store.
>
>> + : [addr] "a" (addr), [new_state] "i" (state));
>
> According to my version of the gcc documentation, the "a" constraint
> means: "Address register (general purpose register except r0)". So it's
> a little bit weird that you use it together with asm("0"). I guess it
> does not really matter in this case ... but while we're at it: I think
> you could simply drop the local "addr" variable here and use paddr
> directly instead.
>
> You could also drop the asm("1") from the extr_state variable as far as
> I can see.
And so I did for both.
Thanks
>
>> + return (unsigned long)extr_state;
>> +}
>
> (remaining parts of the patch look fine to me)
>
> Thomas
>
Attachment:
signature.asc
Description: OpenPGP digital signature
