All of lore.kernel.org
 help / color / mirror / Atom feed
From: Amol Surati <suratiamol@gmail.com>
To: qemu-devel@nongnu.org
Cc: Amol Surati <suratiamol@gmail.com>
Subject: [Qemu-devel] [RFC 0/1] ide: attempt at fixing the bug #1777315.
Date: Mon, 18 Jun 2018 00:05:14 +0530	[thread overview]
Message-ID: <20180617183515.3982-1-suratiamol@gmail.com> (raw)

This is an attempt at fixing the bug #1777315, through code review
alone (i.e. test and debugging are pending.)

The function bmdma_prepare_buf shows that s->io_buffer_size can be
controlled through the PRDs, and it is possible for it to not be a
perfect multiple of the sector size (the function ide_dma_cb assumes
that s->io_buffer_size is a perfect multiple of the sector size.)

For instance, consider a transfer of 630 bytes.
s->nsector should be (probably, I did not verify through test/debug) 2.

Let there be two PRDs, handed over to the bus-master:
prd[0]: len=512, eot=0
prd[1]: len=128, eot=1

The function bmdma_prepare_buf receives limit as 2*512 = 1024. During
the processing of the two PRDs, s->sg.size and s->io_buffer_size both
end up being set to 630.

Within the function ide_dma_cb, s->io_buffer_size >> 9 results in n
being set to 1, which is not greater than s->nsector (which must be
2.)

n * 512 is 512, while s->sg.size is 630; the difference results in
the firing of 'assert(n * 512 == s->sg.size);'.

Alternatively, a single change (discarding the current patch),

'assert(n * 512 == s->sg.size || s->io_buffer_size == s->sg.size);',

could work, provided these two are the only possibilities.




Amol Surati (1):
  ide: bug #1777315: io_buffer_size and sg.size can represent partial
    sector sizes

 hw/ide/core.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

-- 
2.17.1

             reply	other threads:[~2018-06-17 18:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-17 18:35 Amol Surati [this message]
2018-06-17 18:35 ` [Qemu-devel] [RFC 1/1] ide: bug #1777315: io_buffer_size and sg.size can represent partial sector sizes Amol Surati
2018-06-18 18:02   ` Amol Surati
2018-06-18 18:13     ` John Snow
2018-06-18 18:24       ` Amol Surati
2018-06-19  0:14     ` John Snow
2018-06-19  4:01       ` Amol Surati
2018-06-19  8:53         ` [Qemu-devel] [Qemu-block] " Kevin Wolf
2018-06-19 13:45           ` John Snow
2018-06-19 14:34             ` Amol Surati
2018-06-19 21:26               ` Amol Surati
2018-06-19 21:43                 ` John Snow
2018-06-20  0:53                   ` Amol Surati
2018-06-20  1:27                     ` Amol Surati

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180617183515.3982-1-suratiamol@gmail.com \
    --to=suratiamol@gmail.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.