From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Cc: Segher Boessenkool <segher@kernel.crashing.org>,
linuxppc-dev@lists.ozlabs.org,
Anshuman Khandual <anshuman.linux@gmail.com>,
Mike Anderson <andmike@linux.ibm.com>,
Ram Pai <linuxram@us.ibm.com>,
linux-kernel@vger.kernel.org,
Claudio Carvalho <cclaudio@linux.ibm.com>,
Paul Mackerras <paulus@samba.org>, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v2 03/13] powerpc/prom_init: Add the ESM call to prom_init
Date: Fri, 19 Jul 2019 10:09:56 +1000 [thread overview]
Message-ID: <f4cba627-8c40-ce95-0ede-b01edf3546dc@ozlabs.ru> (raw)
In-Reply-To: <875znz3ud7.fsf@morokweng.localdomain>
On 19/07/2019 07:28, Thiago Jung Bauermann wrote:
>
> Hello Segher,
>
> Thanks for your review and suggestions!
>
> Segher Boessenkool <segher@kernel.crashing.org> writes:
>
>> (Sorry to hijack your reply).
>>
>> On Thu, Jul 18, 2019 at 06:11:48PM +1000, Alexey Kardashevskiy wrote:
>>> On 13/07/2019 16:00, Thiago Jung Bauermann wrote:
>>>> From: Ram Pai <linuxram@us.ibm.com>
>>>> +static int enter_secure_mode(unsigned long kbase, unsigned long fdt)
>>>> +{
>>>> + register uint64_t func asm("r3") = UV_ESM;
>>>> + register uint64_t arg1 asm("r4") = (uint64_t)kbase;
>>>> + register uint64_t arg2 asm("r5") = (uint64_t)fdt;
>>>
>>> What does UV do with kbase and fdt precisely? Few words in the commit
>>> log will do.
What about this one? :)
>>>
>>>> +
>>>> + asm volatile("sc 2\n"
>>>> + : "=r"(func)
>>>> + : "0"(func), "r"(arg1), "r"(arg2)
>>>> + :);
>>>> +
>>>> + return (int)func;
>>>
>>> And why "func"? Is it "function"? Weird name. Thanks,
>
> Yes, I believe func is for function. Perhaps ucall would be clearer
> if the variable wasn't reused for the return value as Segher points out.
>
>> Maybe the three vars should just be called "r3", "r4", and "r5" --
>> r3 is used as return value as well, so "func" isn't a great name for it.
>
> Yes, that does seem simpler.
>
>> Some other comments about this inline asm:
>>
>> The "\n" makes the generated asm look funny and has no other function.
>> Instead of using backreferences you can use a "+" constraint, "inout".
>> Empty clobber list is strange.
>> Casts to the return type, like most other casts, are an invitation to
>> bugs and not actually useful.
>>
>> So this can be written
>>
>> static int enter_secure_mode(unsigned long kbase, unsigned long fdt)
>> {
>> register uint64_t r3 asm("r3") = UV_ESM;
>> register uint64_t r4 asm("r4") = kbase;
>> register uint64_t r4 asm("r5") = fdt;
>>
>> asm volatile("sc 2" : "+r"(r3) : "r"(r4), "r"(r5));
>>
>> return r3;
>> }
>
> I'll adopt your version, it is cleaner inded. Thanks for providing it!
>
>> (and it probably should use u64 instead of both uint64_t and unsigned long?)
>
> Almost all of prom_init.c uses unsigned long, with u64 in just a few
> places. uint64_t isn't used anywhere else in the file. I'll switch to
> unsigned long everywhere, since this feature is only for 64 bit.
>
--
Alexey
next prev parent reply other threads:[~2019-07-19 0:10 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-13 6:00 [PATCH v2 00/13] Secure Virtual Machine Enablement Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 01/13] powerpc/pseries: Introduce option to build secure virtual machines Thiago Jung Bauermann
2019-07-13 6:00 ` [RFC PATCH v2 02/13] powerpc: Add support for adding an ESM blob to the zImage wrapper Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 03/13] powerpc/prom_init: Add the ESM call to prom_init Thiago Jung Bauermann
2019-07-18 8:11 ` Alexey Kardashevskiy
2019-07-18 19:58 ` Segher Boessenkool
2019-07-18 21:28 ` Thiago Jung Bauermann
2019-07-19 0:09 ` Alexey Kardashevskiy [this message]
2019-07-19 0:48 ` Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 04/13] powerpc/pseries/svm: Add helpers for UV_SHARE_PAGE and UV_UNSHARE_PAGE Thiago Jung Bauermann
2019-07-18 8:13 ` Alexey Kardashevskiy
2019-07-18 20:12 ` Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 05/13] powerpc/pseries: Add and use LPPACA_SIZE constant Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 06/13] powerpc/pseries/svm: Use shared memory for LPPACA structures Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 07/13] powerpc/pseries/svm: Use shared memory for Debug Trace Log (DTL) Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 08/13] powerpc/pseries/svm: Unshare all pages before kexecing a new kernel Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 09/13] powerpc/pseries/svm: Export guest SVM status to user space via sysfs Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 10/13] powerpc/pseries/svm: Disable doorbells in SVM guests Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 11/13] powerpc/pseries/iommu: Don't use dma_iommu_ops on secure guests Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 12/13] powerpc/pseries/svm: Force SWIOTLB for " Thiago Jung Bauermann
2019-07-13 6:00 ` [PATCH v2 13/13] powerpc/configs: Enable secure guest support in pseries and ppc64 defconfigs Thiago Jung Bauermann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f4cba627-8c40-ce95-0ede-b01edf3546dc@ozlabs.ru \
--to=aik@ozlabs.ru \
--cc=andmike@linux.ibm.com \
--cc=anshuman.linux@gmail.com \
--cc=bauerman@linux.ibm.com \
--cc=cclaudio@linux.ibm.com \
--cc=hch@lst.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=linuxram@us.ibm.com \
--cc=paulus@samba.org \
--cc=segher@kernel.crashing.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).