linux-kernel - Re: [PATCH v11 10/26] gunyah: vm_mgr: Introduce basic VM Manager

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <030888c7-1e32-1663-589a-55e998963ab7@quicinc.com>
Date:   Tue, 11 Apr 2023 13:48:43 -0700
From:   Elliot Berman <quic_eberman@...cinc.com>
To:     Alex Elder <elder@...aro.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Jonathan Corbet <corbet@....net>,
        "Prakruthi Deepak Heragu" <quic_pheragu@...cinc.com>
CC:     Murali Nalajala <quic_mnalajal@...cinc.com>,
        Trilok Soni <quic_tsoni@...cinc.com>,
        Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
        Carl van Schaik <quic_cvanscha@...cinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Bjorn Andersson <andersson@...nel.org>,
        "Konrad Dybcio" <konrad.dybcio@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Bagas Sanjaya <bagasdotme@...il.com>,
        Will Deacon <will@...nel.org>, Andy Gross <agross@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Jassi Brar <jassisinghbrar@...il.com>,
        <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v11 10/26] gunyah: vm_mgr: Introduce basic VM Manager



On 3/31/2023 7:25 AM, Alex Elder wrote:
> On 3/3/23 7:06 PM, Elliot Berman wrote:
>> @@ -129,6 +131,7 @@ struct gh_rm_connection {
>>    * @cache: cache for allocating Tx messages
>>    * @send_lock: synchronization to allow only one request to be sent 
>> at a time
>>    * @nh: notifier chain for clients interested in RM notification 
>> messages
>> + * @miscdev: /dev/gunyah
>>    */
>>   struct gh_rm {
>>       struct device *dev;
>> @@ -145,6 +148,8 @@ struct gh_rm {
>>       struct kmem_cache *cache;
>>       struct mutex send_lock;
>>       struct blocking_notifier_head nh;
>> +
>> +    struct miscdevice miscdev;
>>   };
>>   /**
>> @@ -593,6 +598,21 @@ void gh_rm_put(struct gh_rm *rm)
>>   }
>>   EXPORT_SYMBOL_GPL(gh_rm_put);
> 
> I feel like /dev/gunyah code would more appropriately be found
> in "vm_mgr.c".  All gh_dev_ioctl() does is call the function
> defined there, and it's therefore a VM-oriented rather than
> resource-oriented device.

I'd like to keep the gh_dev_ioctl where it is because it keeps the 
struct gh_rm explicitly private to rsc_mgr.c and thinking this helps 
keep the design cleaner long term by preventing new members from 
sneaking into struct gh_rm.

>> +
>> +static long gh_dev_ioctl_create_vm(struct gh_rm *rm, unsigned long arg)
>> +{
>> +    struct gh_vm *ghvm;
>> +    struct file *file;
>> +    int fd, err;
>> +
>> +    /* arg reserved for future use. */
> 
> Do you have a clear idea of how this might be used in the future?

Not yet. I have some vague ideas to use it as a enumeration of "special" 
VM types. We might have special number for VMs which use "protected VM 
firmware"  for the Android boot flow, another number for the "Trusted UI 
VM", another for "OEM VM", etc. Passing 0 would always be the 
unauthenticated VM which we are creating today.

We're considering bumping the info to a separate ioctl since additional 
info needs to be passed from userspace to configure the VM. Userspace 
would do GH_CREATE_VM(). Another ioctl like GH_VM_SET_PVMFW_ADDRESS() 
would imply that the VM uses the protected VM firmware for the Android 
boot flow. Another ioctl call would be used to imply the "Trusted UI 
VM". In any case, we're still in early design phase.

> 
> I was thinking you could silently ignore the argument value, but
> I suppose if it *does* get used in the future, you want the caller
> to know it's being ignored.  (Is that right?)
> 

That's right.

Thanks,
Elliot