Re: [PATCH v8] drivers/misc: sysgenid: add system generation id driver

From: Greg KH
Date: Tue Mar 23 2021 - 08:56:44 EST


On Mon, Mar 08, 2021 at 05:03:58PM +0100, Alexander Graf wrote:
>
>
> On 08.03.21 15:36, Greg KH wrote:
> >
> > On Mon, Mar 08, 2021 at 04:18:03PM +0200, Adrian Catangiu wrote:
> > > +static struct miscdevice sysgenid_misc = {
> > > + .minor = MISC_DYNAMIC_MINOR,
> > > + .name = "sysgenid",
> > > + .fops = &fops,
> > > +};
> >
> > Much cleaner, but:
> >
> > > +static int __init sysgenid_init(void)
> > > +{
> > > + int ret;
> > > +
> > > + sysgenid_data.map_buf = get_zeroed_page(GFP_KERNEL);
> > > + if (!sysgenid_data.map_buf)
> > > + return -ENOMEM;
> > > +
> > > + atomic_set(&sysgenid_data.generation_counter, 0);
> > > + atomic_set(&sysgenid_data.outdated_watchers, 0);
> > > + init_waitqueue_head(&sysgenid_data.read_waitq);
> > > + init_waitqueue_head(&sysgenid_data.outdated_waitq);
> > > + spin_lock_init(&sysgenid_data.lock);
> > > +
> > > + ret = misc_register(&sysgenid_misc);
> > > + if (ret < 0) {
> > > + pr_err("misc_register() failed for sysgenid\n");
> > > + goto err;
> > > + }
> > > +
> > > + return 0;
> > > +
> > > +err:
> > > + free_pages(sysgenid_data.map_buf, 0);
> > > + sysgenid_data.map_buf = 0;
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void __exit sysgenid_exit(void)
> > > +{
> > > + misc_deregister(&sysgenid_misc);
> > > + free_pages(sysgenid_data.map_buf, 0);
> > > + sysgenid_data.map_buf = 0;
> > > +}
> > > +
> > > +module_init(sysgenid_init);
> > > +module_exit(sysgenid_exit);
> >
> > So you do this for any bit of hardware that happens to be out there?
> > Will that really work? You do not have any hwid to trigger off of to
> > know that this is a valid device you can handle?
>
> The interface is already useful in a pure container context where the
> generation change request is triggered by software.
>
> And yes, there are hardware triggers, but Michael was quite unhappy about
> potential races between VMGenID change and SysGenID change and thus wanted
> to ideally separate the interfaces. So we went ahead and isolated the
> SysGenID one, as it's already useful as is.
>
> Hardware drivers to inject change events into SysGenID can then follow
> later, for all different hardware platforms. But SysGenID as in this patch
> is a completely hardware agnostic concept.

Ok, this is going to play havoc with fuzzers and other "automated
testers", should be fun to watch! :)

Let's queue this up and see what happens...

thanks,

greg k-h