| 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
| ||
|
Date: Thu, 6 Aug 2015 16:28:22 +0200
From: Andrew Jones <drjones@...hat.com>
To: Marc Marí <markmb@...hat.com>
Cc: linux-kernel@...r.kernel.org, Stefan Hajnoczi <stefanha@...il.com>,
"Kevin O'Connor" <kevin@...onnor.net>,
Gerd Hoffmann <kraxel@...hat.com>, Laszlo <lersek@...hat.com>
Subject: Re: [PATCH] QEMU fw_cfg DMA interface documentation
On Thu, Aug 06, 2015 at 04:19:18PM +0200, Marc Marí wrote:
> On Thu, 6 Aug 2015 16:08:49 +0200
> Andrew Jones <drjones@...hat.com> wrote:
>
> > On Thu, Aug 06, 2015 at 01:03:07PM +0200, Marc Marí wrote:
> > > Add fw_cfg DMA interface specfication in the fw_cfg documentation.
> > >
> > > Signed-off-by: Marc Marí <markmb@...hat.com>
> > > ---
> > > Documentation/devicetree/bindings/arm/fw-cfg.txt | 36
> > > ++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/arm/fw-cfg.txt
> > > b/Documentation/devicetree/bindings/arm/fw-cfg.txt index
> > > 953fb64..c880eec 100644 ---
> > > a/Documentation/devicetree/bindings/arm/fw-cfg.txt +++
> > > b/Documentation/devicetree/bindings/arm/fw-cfg.txt @@ -49,6 +49,41
> > > @@ The guest kernel is not expected to use these registers
> > > (although it is certainly allowed to); the device tree bindings are
> > > documented here because this is where device tree bindings reside
> > > in general. +Starting from revision 2, a DMA interface has also
> > > been added. This can be used +through a write-only, 64-bit wide
> > > address register. +
> > > +In this register, a pointer to a FWCfgDmaAccess structure can be
> > > written, in +big endian mode. This is the format of the
> > > FWCfgDmaAccess structure:
> > s/mode/format/
> > > +
> > > +typedef struct FWCfgDmaAccess {
> > > + uint64_t address;
> > > + uint32_t length;
> > > + uint32_t control;
> > > +} FWCfgDmaAccess;
> > > +
> > > +Once the address to this structure has been written, an DMA
> > > operation is +started. If the "control" field has value 2, a read
> > > operation will be performed. +"length" bytes for the current
> > > selector and offset will be mapped into the +address specified by
> > > the "address" field. +
> > > +If the field "address" has value 0, the read is considered a skip,
> > > and +the data is not copied anywhere, but the offset is still
> > > incremented.
> >
> > So we can't DMA to address == 0? That might not generally be a good
> > idea, but why limit ourselves? Can't we add another control input for
> > skip instead? Actually, what inputs are accepted now? READ == 2,
> > WRITE? ??
> >
>
> Write was already disabled for PIO:
>
> static void fw_cfg_write(FWCfgState *s, uint8_t value)
> {
> /* nothing, write support removed in QEMU v2.4+ */
> }
>
> > > +
> > > +To check result, read the control register:
> > > + error bit set -> something went wrong.
> > echo Stefan's which bit question, and also what types of errors? If
> > there are many, then how about an error bit, plus field of bits for
> > an error code?
> >
>
> Bit 0. At this moment the only error possible is DMA mapping failure.
> But its true that it might be useful to have some bits in the control
> field or another field to indicate the type of error, in case of future
> extensions.
>
> > > + all bits cleared -> transfer finished successfully.
> > > + otherwise -> transfer still in progress (doesn't happen
> > > + today due to implementation not being
> > > async,
> > > + but may in the future).
> > I don't think we need to point out that this isn't implemented yet,
> > but may be in the future. If that's how it may work, then let's just
> > document it. And why not specify an in-progress bit?
>
> Is this a feature we will want?
I don't know. Need firmware person like Laszlo to give an opinion. Maybe
he'd want OVMF/AAVMF to be able to output progress while transferring,
to keep users more patient?
>
> > > +
> > > +Target address goes up and transfer length goes down as the
> > > transfer +happens, so after a successful transfer the length
> > > register is zero +and the address register points right after the
> > > memory block written. +
> > > +If a partial transfer happened before an error occured the address
> > > and +length registers indicate how much data has been transfered
> > > +successfully.
> > > +
> > > Required properties:
> > >
> > > - compatible: "qemu,fw-cfg-mmio".
> > > @@ -56,6 +91,7 @@ Required properties:
> > > - reg: the MMIO region used by the device.
> > > * Bytes 0x0 to 0x7 cover the data register.
> > > * Bytes 0x8 to 0x9 cover the selector register.
> > > + * From revision 2: Bytes 0xa to 0x11 cover the DMA address
> > > register.
> >
> > I think we should naturally align the register, i.e we can reserve
> > bytes 0xa - 0xf, and then put the DMA register at 0x10.
> >
> > > * Further registers may be appended to the region in case of
> > > future interface revisions / feature bits.
> > >
> > > --
> > > 2.4.3
> > >
> >
>
> Thanks
> Marc
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists