[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] Add AT24Cxx I2C EEPROM device model
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] Add AT24Cxx I2C EEPROM device model |
Date: |
Tue, 20 Nov 2012 15:40:29 +0100 |
On Tue, Nov 20, 2012 at 3:38 PM, Jan Kiszka <address@hidden> wrote:
> On 2012-11-20 15:06, Stefan Hajnoczi wrote:
>> On Tue, Nov 20, 2012 at 02:37:07PM +0100, Jan Kiszka wrote:
>>> On 2012-11-20 14:27, Stefan Hajnoczi wrote:
>>>> On Mon, Nov 19, 2012 at 03:24:39PM +0100, Jan Kiszka wrote:
>>>>> +static void at24_flush_transfer_buffer(AT24State *s)
>>>>> +{
>>>>> + if (s->cached_sector < 0 || !s->cache_dirty) {
>>>>> + return;
>>>>> + }
>>>>> + bdrv_write(s->bs, s->cached_sector, s->sector_buffer, 1);
>>>> [...]
>>>>> +static int at24_cache_sector(AT24State *s, int sector)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + if (s->cached_sector == sector) {
>>>>> + return 0;
>>>>> + }
>>>>> + ret = bdrv_read(s->bs, sector, s->sector_buffer, 1);
>>>>
>>>> Can you use bdrv_aio_writev()/bdrv_aio_readv()? We should avoid adding
>>>> new synchronous block I/O. Because it forces us to run a nested event
>>>> loop that blocks the guest until I/O completes.
>>>
>>> The call is synchronous as the I2C bus model is as well. How do I model
>>> this with bdrv_aio_*?
>>
>> The bus model needs to be asynchronous.
>
> That's not easy, even in theory. I2C allows a slave to defer its answer
> by holding the clock line down, but not all masters support this, and
> some (e.g. SMBus) even demand an upper limit. So we risk timeouts of the
> guest when deferring normally synchronous accesses like on these EEPROMs.
>
> IOW: There will likely be a need for synchronously waiting on block
> layer I/O requests also in the future.
How large are the EEPROMs? Perhaps the data should be in memory?
Stefan
Re: [Qemu-devel] [PATCH 0/2] i2c: Add AT24Cxx EEPROM model, Stefan Weil, 2012/11/19