virtio — archive
[Date Prev]
| [Thread Prev]
| [Thread Next]
| [Date Next]
— [Date Index]
| [Thread Index]
| [Month Index]
| [List Home]
Re: [virtio] [PATCH] pci: missing documentation for dealing with 64 bit config fields
On Sun, Apr 12, 2015 at 03:38:07PM +0000, James Bottomley wrote:
> On Sun, 2015-04-12 at 17:28 +0200, Michael S. Tsirkin wrote: >
> On Tue, Apr 07, 2015 at 10:40:50PM +0000, James Bottomley wrote: > >
> On Tue, 2015-04-07 at 12:02 +0200, Michael S. Tsirkin wrote: > > >
> On Tue, Apr 07, 2015 at 11:19:08AM +0200, Michael S. Tsirkin wrote: > > > >
> On Mon, Apr 06, 2015 at 06:17:56PM +0000, James Bottomley wrote: > > > > >
> On Thu, 2015-04-02 at 12:23 +0200, Cornelia Huck wrote: > > > > > >
> On Thu, 2 Apr 2015 12:06:41 +0200 > > > > > > > "Michael S. Tsirkin" <[email protected]> wrote: > > > > > > > > > > > > > >
> On Thu, Apr 02, 2015 at 11:01:45AM +0200, Cornelia Huck wrote: > > > > > > > >
> On Wed, 1 Apr 2015 20:42:13 +0200 > > > > > > > > > "Michael S. Tsirkin" <[email protected]> wrote: > > > > > > > > > > > > > > > > > +For device configuration access, the driver MUST use 8-bit wide > > > > > > > > > > +accesses for 8-bit wide fields, 16-bit wide and aligned accesses > > > > > > > > > > +for 16-bit wide fields and 32-bit wide and aligned accesses for > > > > > > > > > > +32-bit and 64-bit wide fields. For 64-bit fields, the driver MAY > > > > > > > > > > +access each of the high and low 32-bit parts of the field > > > > > > > > > > +independently. > > > > > > > > > > > > > > > > >
> Doesn't the last sentence follow from "use 32-bit wide accesses" > > > > > > > >
> already? > > > > > > > > > > > > > > >
> Someone might assume that driver must always access low part, > > > > > > >
> then high part. Last sentence says it does not have to. > > > > > > > > > > > > > > > > > > > > >
> OK, that clears it up then. > > > > > > > > > > >
> Not entirely. There's a hidden gotcha here in terms of fields which > > > > >
> perform actions. For instance the status register is used to transmit > > > > >
> actions and information from the guest to the host. Problems happen if > > > > >
> any action register is wider than 32 bits, because a 32 bit OS cannot > > > > >
> now set it atomically. > > > > > > > > > > >
> There a various ways around this: we could say no action register may > > > > >
> ever be wider than 32 bits (easy) or we could prescribe a mechanism for > > > > >
> using 64 bit action registers on 32 bits (locking + defined initiation, > > > > >
> say writing to the lower 32 bits triggers the action, so on 32 bit > > > > >
> architectures, you lock, write upper, then write lower, then unlock). > > > > > > > > > > >
> I'd actually say it's useful clarification to code the former into the > > > > >
> Spec until such time as someone finds a need for a 64 bit action > > > > >
> register. > > > > > > > > > > >
> James > > > > > > > > > > > > > > >
> This seems to belong to appendix B: creating new device types. > > > >
> We already have this text there: > > > >
> Remember that configuration fields over 32 bits wide might not be > > > >
> atomically writable by the driver. > > > > > > > > >
> Is this sufficient? If not, we can add > > > >
> Therefore, it is generally not a good idea to have > > > >
> such writeable fields, and especially to have > > > >
> driver writes into such fields trigger device actions". > > > > > > > > >
> Is this what you had in mind? > > > > > > >
> If yes I'd prefer that you change your vote to yes, > > >
> with a comment, and open another issue to add a note. > > > > >
> Well, I'd prefer the updates went in atomically, but I think this will > >
> work as a compromise. > > > > >
> James > > > > > >
> Created VIRTIO-140 for this. >
> Will post a patch when I can post to the list (still having trouble with
> our email rebranding). >
> James
>
Since you are a member of the TC, I don't think there are any IPR issues if you just send any email to me, I'll forward. OTOH if you just want that text added, I can send that patch myself.
[Date Prev]
| [Thread Prev]
| [Thread Next]
| [Date Next]
— [Date Index]
| [Thread Index]
| [Month Index]
| [List Home]