[SeaBIOS] [PATCH] QEMU DMA: Add ability to write back to fw_cfg file

Laszlo Ersek lersek at redhat.com
Fri Jan 20 18:29:35 CET 2017


On 01/20/17 17:58, Kevin O'Connor wrote:
> On Fri, Jan 20, 2017 at 05:39:58PM +0100, Laszlo Ersek wrote:
>> On 01/20/17 17:08, Kevin O'Connor wrote:
>>> On Thu, Jan 19, 2017 at 10:20:50PM -0800, ben at skyportsystems.com wrote:
>>>> From: Ben Warren <ben at skyportsystems.com>
>>>>
>>>> This adds a new command to the DMA protocol.  The command allows the
>>>> memory allocation of a fw_cfg file by BIOS and subsequent return of
>>>> the allocated address to QEMU.  The initial use case is for Windows VM
>>>> Generation ID, where QEMU needs to change the contents of fw_cfg
>>>> data at runtime, while still having BIOS allocate and manage the memory.
>>>
>>> Thanks - see my comments below.
>>>
>>> [...]
>>>> --- a/src/fw/paravirt.c
>>>> +++ b/src/fw/paravirt.c
>>>> @@ -253,6 +253,20 @@ qemu_cfg_read(void *buf, int len)
>>>>  }
>>>>  
>>>>  static void
>>>> +qemu_cfg_write(void *buf, int len)
>>>> +{
>>>> +    if (len == 0) {
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    if (qemu_cfg_dma_enabled()) {
>>>> +        qemu_cfg_dma_transfer(buf, len, QEMU_CFG_DMA_CTL_WRITE);
>>>> +    } else {
>>>> +        outsb(PORT_QEMU_CFG_DATA, buf, len);
>>>> +    }
>>>> +}
>>>> +
>>>> +static void
>>>>  qemu_cfg_skip(int len)
>>>>  {
>>>>      if (len == 0) {
>>>> @@ -280,6 +294,19 @@ qemu_cfg_read_entry(void *buf, int e, int len)
>>>>      }
>>>>  }
>>>>  
>>>> +static void
>>>> +qemu_cfg_write_entry(void *buf, int e, int len)
>>>> +{
>>>> +    if (qemu_cfg_dma_enabled()) {
>>>> +        u32 control = (e << 16) | QEMU_CFG_DMA_CTL_SELECT
>>>> +                        | QEMU_CFG_DMA_CTL_WRITE;
>>>> +        qemu_cfg_dma_transfer(buf, len, control);
>>>> +    } else {
>>>> +        qemu_cfg_select(e);
>>>> +        qemu_cfg_write(buf, len);
>>>> +    }
>>>> +}
>>>> +
>>>>  struct qemu_romfile_s {
>>>>      struct romfile_s file;
>>>>      int select, skip;
>>>> @@ -303,6 +330,24 @@ qemu_cfg_read_file(struct romfile_s *file, void *dst, u32 maxlen)
>>>>      return file->size;
>>>>  }
>>>>  
>>>> +static int
>>>> +qemu_cfg_write_file(void *src, struct romfile_s *file, u32 maxlen)
>>>> +{
>>>> +    if (file->size < maxlen)
>>>> +        return -1;
>>>> +    struct qemu_romfile_s *qfile;
>>>> +    qfile = container_of(file, struct qemu_romfile_s, file);
>>>> +    if (qfile->skip == 0) {
>>>> +        /* Do it in one transfer */
>>>> +        qemu_cfg_write_entry(src, qfile->select, file->size);
>>>> +    } else {
>>>> +        qemu_cfg_select(qfile->select);
>>>> +        qemu_cfg_skip(qfile->skip);
>>>> +        qemu_cfg_write(src, file->size);
>>>> +    }
>>>> +    return file->size;
>>>> +}
>>>
>>> I'd prefer if we could break this patch into two parts - one part to
>>> add the write interface to fw_cfg and one part to add
>>> ALLOCATE_RET_ADDR.  I'm fine with both interfaces, but the QEMU parts
>>> still need to be committed prior to committing to SeaBIOS.  Having the
>>> patch in separate parts will make it both easier to review and commit
>>> at their appropriate times.
>>
>> The fw_cfg write support is now upstream (QEMU commit
>> baf2d5bfbac015b27f4db74feab235e167df0c84, "fw-cfg: support writeable
>> blobs").
> 
> Thanks.  Is it correct that the QEMU write support is only possible
> with the DMA interface?  If so, I'd request that Ben's patch also be
> updated to not issue an outsb().  Preferably it should just fail at
> the start of qemu_cfg_write_file() if the DMA interface isn't
> available.

It varies across QEMU versions. Up to and including QEMU 2.3, IO port
writes were supported. In the [2.4, 2.8] range, inclusive, there was no
write support. Starting with 2.9, only the DMA interface supports writes
(and writeability is tracked independently of the numerical value of the
given fw_cfg key).

Today the idea is basically, if you have found an fw_cfg file by name
that you know is writeable, as opposed to a predefined numerical
selector value, then you can write to it with the DMA interface. The
case where the writeable, named file in question is found, but the DMA
interface is missing (so you'd have to revert to IO port write), is
nonexistent.

So I agree that the IO port-based write is practically dead code here.

Thanks
Laszlo



More information about the SeaBIOS mailing list