| Message ID | 8738wifwb2.fsf@rustcorp.com.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2013-02-27 at 10:43 +1030, Rusty Russell wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > Hi, > > > > I have noticed that virtio-rng only returns zero for kernels >= 2.6.33 > > built with CONFIG_HW_RANDOM=m. This is a bit much too predictable for a > > random generator ;-). > > Wow. Fortunately, all of SLES, RHEL, Ubuntu or Fedora set > CONFIG_HW_RANDOM=y. What do they know that we don't? > > Oops, looks like Debian testing: config-3.2.0-4-amd64:CONFIG_HW_RANDOM=m [...] *Groan*. I'll change the configuration for now, but trust this will be fixed in virtio-rng later. Thanks for letting me know. Ben.
On Wed, Feb 27, 2013 at 10:43:37AM +1030, Rusty Russell wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > Hi, > > > > I have noticed that virtio-rng only returns zero for kernels >= 2.6.33 > > built with CONFIG_HW_RANDOM=m. This is a bit much too predictable for a > > random generator ;-). > > Wow. Fortunately, all of SLES, RHEL, Ubuntu or Fedora set > CONFIG_HW_RANDOM=y. What do they know that we don't? > > Oops, looks like Debian testing: config-3.2.0-4-amd64:CONFIG_HW_RANDOM=m > > > The reason for that is virtio expects guest real addresses, while > > rng_core.ko (ie when built as a module) is passing a vmalloced buffer > > to the virtio-rng read function, declared as such: > > > > static u8 rng_buffer[SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES] > > __cacheline_aligned; > > Yuck... It would be nice if this has oopsed. Jens, what about this patch? > > Cheers, > Rusty. > > Subject: scatterlist: sg_set_buf() argument must be in linear mapping. > > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h > index 4bd6c06..9365375 100644 > --- a/include/linux/scatterlist.h > +++ b/include/linux/scatterlist.h > @@ -111,6 +111,9 @@ static inline struct page *sg_page(struct scatterlist *sg) > static inline void sg_set_buf(struct scatterlist *sg, const void *buf, > unsigned int buflen) > { > +#ifdef CONFIG_DEBUG_SG > + BUG_ON(!virt_addr_valid(buf)); > +#endif > sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf)); > } > I confirm this patch catches the issue. Thanks. Tested-by: Aurelien Jarno <aurelien@aurel32.net>
On Wed, Feb 27 2013, Rusty Russell wrote: > Aurelien Jarno <aurelien@aurel32.net> writes: > > Hi, > > > > I have noticed that virtio-rng only returns zero for kernels >= 2.6.33 > > built with CONFIG_HW_RANDOM=m. This is a bit much too predictable for a > > random generator ;-). > > Wow. Fortunately, all of SLES, RHEL, Ubuntu or Fedora set > CONFIG_HW_RANDOM=y. What do they know that we don't? > > Oops, looks like Debian testing: config-3.2.0-4-amd64:CONFIG_HW_RANDOM=m > > > The reason for that is virtio expects guest real addresses, while > > rng_core.ko (ie when built as a module) is passing a vmalloced buffer > > to the virtio-rng read function, declared as such: > > > > static u8 rng_buffer[SMP_CACHE_BYTES < 32 ? 32 : SMP_CACHE_BYTES] > > __cacheline_aligned; > > Yuck... It would be nice if this has oopsed. Jens, what about this patch? > > Cheers, > Rusty. > > Subject: scatterlist: sg_set_buf() argument must be in linear mapping. > > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h > index 4bd6c06..9365375 100644 > --- a/include/linux/scatterlist.h > +++ b/include/linux/scatterlist.h > @@ -111,6 +111,9 @@ static inline struct page *sg_page(struct scatterlist *sg) > static inline void sg_set_buf(struct scatterlist *sg, const void *buf, > unsigned int buflen) > { > +#ifdef CONFIG_DEBUG_SG > + BUG_ON(!virt_addr_valid(buf)); > +#endif > sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf)); > } Looks good to me, in lieu of being able to return an error. Want me to queue it up?
Jens Axboe <axboe@kernel.dk> writes: > On Wed, Feb 27 2013, Rusty Russell wrote: >> Subject: scatterlist: sg_set_buf() argument must be in linear mapping. >> >> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> >> >> diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h >> index 4bd6c06..9365375 100644 >> --- a/include/linux/scatterlist.h >> +++ b/include/linux/scatterlist.h >> @@ -111,6 +111,9 @@ static inline struct page *sg_page(struct scatterlist *sg) >> static inline void sg_set_buf(struct scatterlist *sg, const void *buf, >> unsigned int buflen) >> { >> +#ifdef CONFIG_DEBUG_SG >> + BUG_ON(!virt_addr_valid(buf)); >> +#endif >> sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf)); >> } > > Looks good to me, in lieu of being able to return an error. Want me to > queue it up? Please... it'll catch me the next time I make the same mistake :) (Though the static-definitions-in-modules-on-most-archs is a pretty nasty corner case). Thanks, Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h index 4bd6c06..9365375 100644 --- a/include/linux/scatterlist.h +++ b/include/linux/scatterlist.h @@ -111,6 +111,9 @@ static inline struct page *sg_page(struct scatterlist *sg) static inline void sg_set_buf(struct scatterlist *sg, const void *buf, unsigned int buflen) { +#ifdef CONFIG_DEBUG_SG + BUG_ON(!virt_addr_valid(buf)); +#endif sg_set_page(sg, virt_to_page(buf), buflen, offset_in_page(buf)); }