Re: [Xen-devel] [PATCH v5 01/10] xen/arm: add generic TEE mediator framework

From: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
To: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	"tee-dev@lists.linaro.org" <tee-dev@lists.linaro.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [Xen-devel] [PATCH v5 01/10] xen/arm: add generic TEE mediator framework
Date: Tue, 11 Jun 2019 18:18:37 +0000	[thread overview]
Message-ID: <87y3286l50.fsf@epam.com> (raw)
In-Reply-To: <73a3c967-66f8-5d26-f657-5093c25cc76b@arm.com>

Hi Julien,

Julien Grall writes:

> On 06/06/2019 17:02, Julien Grall wrote:
>> Hi Volodymyr,
>>
>> On 5/21/19 10:25 PM, Volodymyr Babchuk wrote:
>>> +static inline bool tee_handle_call(struct cpu_user_regs *regs)
>>> +{
>>> + return false;
>>> +}
>>> +
>>> +static inline int tee_domain_init(struct domain *d, uint16_t tee_type)
>>> +{
>>> + return -ENODEV;
>>> +}
>>
>> I had a report that Xen fails to boot with this series and
>> !CONFIG_TEE. This is because you return an error here in all the
>> case some domain creation will always fail.
Thanks for reporting. I forgot to test that case :(

>> Instead this should check that tee_type is always NONE or else return an error.
>>
>> Also, please at least check that your series does not break boot
>> when CONFIG_TEE is not selected. It would also be ideal (but not
>> mandatory) if you can check that it does not break on non-OPTEE
>> platform when !CONFIG_TEE is selected.
>
> I just realized this paragraph may not be clear. What I meant is we
> need to at least test there are no regression when booting when with
> CONFIG_TEE=n.
>
> For CONFIG_TEE=y, it would be good to test that it still boots on
> platform not providing OP-TEE. This is not critical because the config
> cannot be selected without CONFIG_XEN_EXPERT=y.
I fixed CONFIG_TEE=n issue in the new version, which I'm going to send
later today.

Also I made optee_probe() (with CONFIG_OPTEE=y of course) to return
false to emulated platform without OP-TEE. System boots and works as
usual.

Also I addressed your other comments for this patch.

--
Best regards,Volodymyr Babchuk
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel