Re: [PATCH kvm-unit-tests 8/9] s390x: add vmalloc support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11.01.2018 13:07, David Hildenbrand wrote:
> 
>>>  
>>> +static inline void lctlg(int cr, uint64_t value)
>>> +{
>>> +	asm volatile(
>>> +		"	lctlg	%1,%1,%0\n"
>>> +		: : "Q" (value), "i" (cr));
>>
>> Doesn't the compiler complain here? I though that "i" is only possible
>> with constants? I guess the compiler is smart enough to replace it with
>> the right constants, since this is an inline function. But maybe better
>> play safe and turn this into a macro instead.
> 
> This works as it gets inlined. asm macros is frowned upon, no?

I'm just afraid that it is specific to the compiler version whether this
works with an inline function or not. But ok, let's keep it this way
first, and if we later hit a compiler where this does not work, we still
can replace it with a macro instead.

>>> +	asm volatile(
>>> +		/* fetch the old PSW masks */
>>> +		"	epsw	%1,%2\n"
>>> +		/* remove the DAT flag first */
>>> +		"	nilh	%1,0xfbff\n"
>>> +		"	or	%3,%3\n"
>>> +		"	jz	disable\n"
>>> +		/* set DAT flag if enable == true */
>>> +		"	oilh	%1,0x0400\n"
>>> +		"	st	%1,0(%0)\n"
>>> +		"disable:\n"
>>
>> Shouldn't the "disable" label be placed before the "st" ?
> 
> yes indeed
>>
>>> +		"	st	%2,4(%0)\n"
>>> +		/* load the target address for our new PSW */
>>> +		"	larl	%1,cont\n"
>>> +		"	stg	%1,8(%0)\n"
>>> +		"	lpswe	0(%0)\n"
>>> +		"cont:\n"
>>> +		: : "a" (&psw), "r" (0), "r" (1), "r" (enable)
>>
>> The usage of "r" (0), "r" (1) looks wrong. I think you should rather put
>> them into the output list instead, since they are modified within the
>> assembler code. Or maybe even better, use fixed registers in the
>> assembler code and mark the corresponding registers in the clobber list.
> 
> I tried to use the output list way and it would not let me specify
> immediates (0). So I have to introduce local variables. Or is there
> another way?

Use "+r"(0) in the output list? ... if that does not work, I think I'd
rather use the clobber list instead.

 Thomas



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux