linux-kernel - Re: [PATCH v2 5/5] venus: register separate driver for firmware device

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 6 Jun 2018 14:41:38 +0900
From:   Tomasz Figa <tfiga@...omium.org>
To:     Stanimir Varbanov <stanimir.varbanov@...aro.org>
Cc:     vgarodia@...eaurora.org, Hans Verkuil <hverkuil@...all.nl>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Andy Gross <andy.gross@...aro.org>, bjorn.andersson@...aro.org,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        linux-soc@...r.kernel.org, devicetree@...r.kernel.org,
        Alexandre Courbot <acourbot@...omium.org>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH v2 5/5] venus: register separate driver for firmware device

On Tue, Jun 5, 2018 at 5:45 PM Stanimir Varbanov
<stanimir.varbanov@...aro.org> wrote:
>
> Cc: Arnd
>
> On 06/05/2018 07:08 AM, Tomasz Figa wrote:
> > On Mon, Jun 4, 2018 at 10:56 PM Stanimir Varbanov
> > <stanimir.varbanov@...aro.org> wrote:
> >>
> >> Hi Tomasz,
> >>
> >> On 06/04/2018 04:18 PM, Tomasz Figa wrote:
> >>> Hi Vikash,
> >>>
> >>> On Sat, Jun 2, 2018 at 5:27 AM Vikash Garodia <vgarodia@...eaurora.org> wrote:
> >>>> +static int __init venus_init(void)
> >>>> +{
> >>>> +       int ret;
> >>>> +
> >>>> +       ret = platform_driver_register(&qcom_video_firmware_driver);
> >>>> +       if (ret)
> >>>> +               return ret;
> >>>
> >>> Do we really need this firmware driver? As far as I can see, the
> >>> approach used here should work even without any driver bound to the
> >>> firmware device.
> >>
> >> We need device/driver bind because we need to call dma_configure() which
> >> internally doing iommus sID parsing.
> >
> > I can see some drivers calling of_dma_configure() directly:
> > https://elixir.bootlin.com/linux/latest/ident/of_dma_configure
> >
> > I'm not sure if it's more elegant, but should at least require less code.
>
> I think that in this case of non-TZ where we do iommu mapping by hand we
> can use shared-dma-pool reserved memory see how venus_boot has been
> implemented in the beginning [1].

I might have misunderstood something, but wasn't the shared-dma-pool
about reserving physical memory, while the venus firmware problem is
about reserving certain range of IOVA?

>
> Arnd what do you think?
>
> Some background, we have a use-case where the memory for firmware needs
> to be mapped by the venus driver by hand instead of TZ firmware calls.
> I.e. we want to support both, iommu mapping from the driver and mapping
> done by TZ firmware. How we will differentiate what mapping (TZ or
> non-TZ) will be used is a separate issue.
>
> >
> > By the way, can we really assume that probe of firmware platform
> > device really completes before we call venus_boot()?
>
> I'd say we cannot.

Looking at current implementation in driver core,
of_platform_populate() would actually trigger a synchronous probe, so
I guess it could work. However, I'm not sure if this is a general
guarantee here or it's an implementation detail that shouldn't be
relied on.

If we end up really need to have this platform_driver, I guess we
could call platform_driver_probe() after of_platform_populate(),
rather than pre-registering the driver. That seems to be the way to
ensure that the probe is synchronous and we can also check that a
matching device was found by the return value.

Best regards,
Tomasz