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: <5a026df6-05d2-42ef-21dd-e0f70071fc90@quicinc.com>
Date:   Fri, 9 Jun 2023 13:00:16 -0700
From:   Elliot Berman <quic_eberman@...cinc.com>
To:     Alex Elder <elder@...aro.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        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>,
        Jonathan Corbet <corbet@....net>,
        Bagas Sanjaya <bagasdotme@...il.com>,
        Andy Gross <agross@...nel.org>,
        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 v13 16/24] virt: gunyah: Translate gh_rm_hyp_resource into
 gunyah_resource



On 6/5/2023 12:49 PM, Alex Elder wrote:
> On 5/9/23 3:47 PM, Elliot Berman wrote:
>> When booting a Gunyah virtual machine, the host VM may gain capabilities
>> to interact with resources for the guest virtual machine. Examples of
>> such resources are vCPUs or message queues. To use those resources, we
>> need to translate the RM response into a gunyah_resource structure which
>> are useful to Linux drivers. Presently, Linux drivers need only to know
>> the type of resource, the capability ID, and an interrupt.
>>
>> On ARM64 systems, the interrupt reported by Gunyah is the GIC interrupt
>> ID number and always a SPI.
>>
>> Signed-off-by: Elliot Berman <quic_eberman@...cinc.com>
> 
> Please zero the automatic variable in the place I suggest it.
> I have two other comments/questions.  Otherwise, this looks good.
> 
> Reviewed-by: Alex Elder <elder@...aro.org>
> 
>> ---

...

>> +struct gh_resource *gh_rm_alloc_resource(struct gh_rm *rm, struct 
>> gh_rm_hyp_resource *hyp_resource)
>> +{
>> +    struct gh_resource *ghrsc;
>> +    int ret;
>> +
>> +    ghrsc = kzalloc(sizeof(*ghrsc), GFP_KERNEL);
>> +    if (!ghrsc)
>> +        return NULL;
>> +
>> +    ghrsc->type = hyp_resource->type;
>> +    ghrsc->capid = le64_to_cpu(hyp_resource->cap_id);
>> +    ghrsc->irq = IRQ_NOTCONNECTED;
>> +    ghrsc->rm_label = le32_to_cpu(hyp_resource->resource_label);
>> +    if (hyp_resource->virq) {
>> +        struct gh_irq_chip_data irq_data = {
>> +            .gh_virq = le32_to_cpu(hyp_resource->virq),
>> +        };
>> +
>> +        ret = irq_domain_alloc_irqs(rm->irq_domain, 1, NUMA_NO_NODE, 
>> &irq_data);
>> +        if (ret < 0) {
>> +            dev_err(rm->dev,
>> +                "Failed to allocate interrupt for resource %d label: 
>> %d: %d\n",
>> +                ghrsc->type, ghrsc->rm_label, ghrsc->irq);
> 
> Is it reasonable to return in this case without indicating to the
> caller that something is wrong?
> 

I wasn't sure what to do here since this is unexpected edge case. Not 
returning would cause a client's "request_irq" to fail down the line if 
the client was interested in the irq. I had picked not to return since 
this error doesn't put us in an unrecoverable state. No one currently 
wants to try to recover from that error, so I'm really just deferring 
the real error handling until later.

I can return ret here.

>> +        } else {
>> +            ghrsc->irq = ret;
>> +        }
>> +    }
>> +
>> +    return ghrsc;

...

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ