On Mon, Mar 11, 2024 at 11:08:06AM +0530, Srivatsa Vaddagiri wrote:
> * Elliot Berman <quic_eberman@xxxxxxxxxxx> [2024-02-22 15:16:35]:
>
> > Gunyah doesn't process the label and that makes it
> > possible for userspace to create multiple resources with the same label.
>
> I think that description conflicts with what is implemented in this patch?
>
> int gunyah_vm_add_resource_ticket(struct gunyah_vm *ghvm,
> struct gunyah_vm_resource_ticket *ticket)
> {
> mutex_lock(&ghvm->resources_lock);
> list_for_each_entry(iter, &ghvm->resource_tickets, vm_list) {
> if (iter->resource_type == ticket->resource_type &&
> iter->label == ticket->label) {
> ret = -EEXIST;
> goto out;
> }
> }
>
>
> //
>
It's a justification for the next sentence in the commit text:
Resource ticket owners need to be prepared for populate to be called
multiple times if userspace created multiple resources with the same
label.
VM manager can make sure that only one entity is going to receive the
resources (the check above you highlighted), but I can't make sure that
it's only going to exactly one resource. We don't currently have a
scenario where we need/want multiple resources with the same label, we
might have that in the future and I didn't want to add that restriction
in common code.
> > @@ -134,6 +246,25 @@ static int gunyah_vm_start(struct gunyah_vm *ghvm)
> > }
> > ghvm->vm_status = GUNYAH_RM_VM_STATUS_READY;
> >
> > + ret = gunyah_rm_get_hyp_resources(ghvm->rm, ghvm->vmid, &resources);
> > + if (ret) {
> > + dev_warn(ghvm->parent,
> > + "Failed to get hypervisor resources for VM: %d\n",
> > + ret);
> > + goto err;
> > + }
>
> Where do we free memory pointed by 'resources' ptr?
>
Ah, I meant to add the __free(kfree) annotation. I've added it.
> > +
> > + for (i = 0, n = le32_to_cpu(resources->n_entries); i < n; i++) {
> > + ghrsc = gunyah_rm_alloc_resource(ghvm->rm,
> > + &resources->entries[i]);
> > + if (!ghrsc) {
> > + ret = -ENOMEM;
> > + goto err;
> > + }
> > +
> > + gunyah_vm_add_resource(ghvm, ghrsc);
> > + }
