* Re: [RFC PATCH 8/8] virtio: net: Add freeze, restore handlers to support S4
From: Amit Shah @ 2011-07-29 4:28 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Virtualization List
In-Reply-To: <20110729041335.GD31888@redhat.com>
On (Fri) 29 Jul 2011 [07:13:35], Michael S. Tsirkin wrote:
> > +static int virtnet_restore(struct virtio_device *vdev)
> > +{
> > + struct virtnet_info *vi = vdev->priv;
> > + struct virtqueue *vqs[3];
> > + vq_callback_t *callbacks[] = { skb_recv_done, skb_xmit_done, NULL};
> > + const char *names[] = { "input", "output", "control" };
> > + int nvqs, err;
> > +
> > + /* We expect two virtqueues, receive then send,
> > + * and optionally control. */
> > + nvqs = virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ? 3 : 2;
> > +
> > + err = vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names);
> > + if (err)
> > + return err;
> > +
> > + vi->rvq = vqs[0];
> > + vi->svq = vqs[1];
> > +
> > + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
> > + vi->cvq = vqs[2];
> > + }
> > +
> > + /* Last of all, set up some receive buffers. */
> > + try_fill_recv(vi, GFP_KERNEL);
>
> should probably update carrier status too.
Will add that.
I think most of these restore routines are incomplete; I just pulled
in the most important things from the probe routines, but before the
final submission I'll do a thorough walk-through and ensure I get all
the bits necessary.
Amit
^ permalink raw reply
* Re: [RFC PATCH 4/8] virtio: console: Ignore port name update request if name already set
From: Michael S. Tsirkin @ 2011-07-29 4:27 UTC (permalink / raw)
To: Amit Shah; +Cc: Virtualization List
In-Reply-To: <20110729042619.GC24868@amit-x200.redhat.com>
On Fri, Jul 29, 2011 at 09:56:19AM +0530, Amit Shah wrote:
> On (Fri) 29 Jul 2011 [07:19:58], Michael S. Tsirkin wrote:
> > On Thu, Jul 28, 2011 at 08:05:09PM +0530, Amit Shah wrote:
> > > We don't allow port name changes dynamically for a port. So any
> > > requests by the host to change the name are ignored.
> >
> > It would be nice to check the name and if
> > changed, print a warning.
>
> But not change it, right?
That's what I meant.
> I don't know what applications expect, but
> I think we should keep the port names stable while they're plugged in.
>
> > > Before this patch, if the hypervisor sent a port name while we had one
> > > set already, we would leak memory equivalent to the size of the old
> > > name.
> > >
> > > This scenario wasn't expected so far, but with the suspend-resume
> > > support, we'll send the VIRTIO_CONSOLE_PORT_READY message after restore,
> > > which can get us into this situation.
> > >
> > > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> >
> > Memleaks are bad. So this is a bugfix we probably should have anyway?
>
> Yes.
>
> Amit
^ permalink raw reply
* Re: [RFC PATCH 4/8] virtio: console: Ignore port name update request if name already set
From: Amit Shah @ 2011-07-29 4:26 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Virtualization List
In-Reply-To: <20110729041958.GE31888@redhat.com>
On (Fri) 29 Jul 2011 [07:19:58], Michael S. Tsirkin wrote:
> On Thu, Jul 28, 2011 at 08:05:09PM +0530, Amit Shah wrote:
> > We don't allow port name changes dynamically for a port. So any
> > requests by the host to change the name are ignored.
>
> It would be nice to check the name and if
> changed, print a warning.
But not change it, right? I don't know what applications expect, but
I think we should keep the port names stable while they're plugged in.
> > Before this patch, if the hypervisor sent a port name while we had one
> > set already, we would leak memory equivalent to the size of the old
> > name.
> >
> > This scenario wasn't expected so far, but with the suspend-resume
> > support, we'll send the VIRTIO_CONSOLE_PORT_READY message after restore,
> > which can get us into this situation.
> >
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
>
> Memleaks are bad. So this is a bugfix we probably should have anyway?
Yes.
Amit
^ permalink raw reply
* Re: [RFC PATCH 3/8] virtio-pci: save/restore config space across S4
From: Amit Shah @ 2011-07-29 4:20 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Virtualization List
In-Reply-To: <20110729040758.GC31888@redhat.com>
On (Fri) 29 Jul 2011 [07:07:58], Michael S. Tsirkin wrote:
> On Thu, Jul 28, 2011 at 08:05:08PM +0530, Amit Shah wrote:
> > The virtio config space doesn't get saved across hibernation; we save it
> > locally and update it after restore.
> >
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> > drivers/virtio/virtio_pci.c | 6 ++++++
> > 1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> > index 579681f..9c37561 100644
> > --- a/drivers/virtio/virtio_pci.c
> > +++ b/drivers/virtio/virtio_pci.c
> > @@ -56,6 +56,10 @@ struct virtio_pci_device
> > unsigned msix_vectors;
> > /* Vectors allocated, excluding per-vq vectors if any */
> > unsigned msix_used_vectors;
> > +
> > + /* Status saved during hibernate/restore */
> > + u8 saved_status;
> > +
> > /* Whether we have vector per vq */
> > bool per_vq_vectors;
> > };
> > @@ -713,6 +717,7 @@ static int virtio_pci_freeze(struct device *dev)
> > drv = container_of(vp_dev->vdev.dev.driver,
> > struct virtio_driver, driver);
> >
> > + vp_dev->saved_status = vp_get_status(&vp_dev->vdev);
> > if (drv && drv->freeze)
> > return drv->freeze(&vp_dev->vdev);
> >
> > @@ -728,6 +733,7 @@ static int virtio_pci_restore(struct device *dev)
> > drv = container_of(vp_dev->vdev.dev.driver,
> > struct virtio_driver, driver);
> >
> > + vp_set_status(&vp_dev->vdev, vp_dev->saved_status);
> > if (drv && drv->restore)
> > return drv->restore(&vp_dev->vdev);
>
> Yes, this looks wrong. Status would typically be DRIVER_OK,
> and it needs to be set after vq setup.
Because I expect this to be carried forward from the hibernated
image.. the feature negotiation, etc., all happened the first time
around, and the driver should just be ready when the image is
restored. I still don't know why the status is reset, and why we'd
need to replay the sequence in probe.
Of course, this patch exists in the current form only because I don't
know the answers yet (as mentioned in 0/8). Once I know the answer,
either we'll have a better patch or a better description for this
patch.
> It also needs to match current state I think:
> e.g. if restore failed we want to set it to FAILED.
If restore fails, we return to a normal bootup sequence. The probe
routine should handle that case normally. However, this is something
that has to be tested to ensure it works fine.
Amit
^ permalink raw reply
* Re: [RFC PATCH 4/8] virtio: console: Ignore port name update request if name already set
From: Michael S. Tsirkin @ 2011-07-29 4:19 UTC (permalink / raw)
To: Amit Shah; +Cc: Virtualization List
In-Reply-To: <eb5ef73c21ccdd2700bc56014a90c878c0540326.1311863438.git.amit.shah@redhat.com>
On Thu, Jul 28, 2011 at 08:05:09PM +0530, Amit Shah wrote:
> We don't allow port name changes dynamically for a port. So any
> requests by the host to change the name are ignored.
It would be nice to check the name and if
changed, print a warning.
>
> Before this patch, if the hypervisor sent a port name while we had one
> set already, we would leak memory equivalent to the size of the old
> name.
>
> This scenario wasn't expected so far, but with the suspend-resume
> support, we'll send the VIRTIO_CONSOLE_PORT_READY message after restore,
> which can get us into this situation.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
Memleaks are bad. So this is a bugfix we probably should have anyway?
> ---
> drivers/char/virtio_console.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index fb68b12..3c10b10 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -1394,6 +1394,13 @@ static void handle_control_message(struct ports_device *portdev,
> break;
> case VIRTIO_CONSOLE_PORT_NAME:
> /*
> + * If we woke up after hibernation, we can get this
> + * again. Skip it in that case.
> + */
> + if (port->name)
> + break;
> +
> + /*
> * Skip the size of the header and the cpkt to get the size
> * of the name that was sent
> */
> --
> 1.7.6
^ permalink raw reply
* Re: [RFC PATCH 8/8] virtio: net: Add freeze, restore handlers to support S4
From: Michael S. Tsirkin @ 2011-07-29 4:13 UTC (permalink / raw)
To: Amit Shah; +Cc: Virtualization List
In-Reply-To: <3d2fd91237cfb3ff1e05e464e305df2711b45885.1311863438.git.amit.shah@redhat.com>
On Thu, Jul 28, 2011 at 08:05:13PM +0530, Amit Shah wrote:
> Remove all the vqs on hibernation and re-create them after restoring
> from a hibernated image. This keeps networking working across
> hibernation.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/net/virtio_net.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 0c7321c..c9aaa8f 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1117,6 +1117,53 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
> free_netdev(vi->dev);
> }
>
> +#ifdef CONFIG_PM
> +static int virtnet_freeze(struct virtio_device *vdev)
> +{
> + struct virtnet_info *vi = vdev->priv;
> +
> + cancel_delayed_work_sync(&vi->refill);
> +
> + /* Free unused buffers in both send and recv, if any. */
> + free_unused_bufs(vi);
> +
> + vdev->config->del_vqs(vi->vdev);
> +
> + while (vi->pages)
> + __free_pages(get_a_page(vi, GFP_KERNEL), 0);
> +
> + return 0;
> +}
> +
> +static int virtnet_restore(struct virtio_device *vdev)
> +{
> + struct virtnet_info *vi = vdev->priv;
> + struct virtqueue *vqs[3];
> + vq_callback_t *callbacks[] = { skb_recv_done, skb_xmit_done, NULL};
> + const char *names[] = { "input", "output", "control" };
> + int nvqs, err;
> +
> + /* We expect two virtqueues, receive then send,
> + * and optionally control. */
> + nvqs = virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ? 3 : 2;
> +
> + err = vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names);
> + if (err)
> + return err;
> +
> + vi->rvq = vqs[0];
> + vi->svq = vqs[1];
> +
> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
> + vi->cvq = vqs[2];
> + }
> +
> + /* Last of all, set up some receive buffers. */
> + try_fill_recv(vi, GFP_KERNEL);
should probably update carrier status too.
> + return 0;
> +}
> +#endif
> +
> static struct virtio_device_id id_table[] = {
> { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> { 0 },
> @@ -1141,6 +1188,10 @@ static struct virtio_driver virtio_net_driver = {
> .probe = virtnet_probe,
> .remove = __devexit_p(virtnet_remove),
> .config_changed = virtnet_config_changed,
> +#ifdef CONFIG_PM
> + .freeze = virtnet_freeze,
> + .restore = virtnet_restore,
> +#endif
> };
>
> static int __init init(void)
> --
> 1.7.6
^ permalink raw reply
* Re: [RFC PATCH 3/8] virtio-pci: save/restore config space across S4
From: Michael S. Tsirkin @ 2011-07-29 4:07 UTC (permalink / raw)
To: Amit Shah; +Cc: Virtualization List
In-Reply-To: <124c8fbc2974d0353cac204d5d9ff35a6a6a0e13.1311863438.git.amit.shah@redhat.com>
On Thu, Jul 28, 2011 at 08:05:08PM +0530, Amit Shah wrote:
> The virtio config space doesn't get saved across hibernation; we save it
> locally and update it after restore.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> drivers/virtio/virtio_pci.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index 579681f..9c37561 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -56,6 +56,10 @@ struct virtio_pci_device
> unsigned msix_vectors;
> /* Vectors allocated, excluding per-vq vectors if any */
> unsigned msix_used_vectors;
> +
> + /* Status saved during hibernate/restore */
> + u8 saved_status;
> +
> /* Whether we have vector per vq */
> bool per_vq_vectors;
> };
> @@ -713,6 +717,7 @@ static int virtio_pci_freeze(struct device *dev)
> drv = container_of(vp_dev->vdev.dev.driver,
> struct virtio_driver, driver);
>
> + vp_dev->saved_status = vp_get_status(&vp_dev->vdev);
> if (drv && drv->freeze)
> return drv->freeze(&vp_dev->vdev);
>
> @@ -728,6 +733,7 @@ static int virtio_pci_restore(struct device *dev)
> drv = container_of(vp_dev->vdev.dev.driver,
> struct virtio_driver, driver);
>
> + vp_set_status(&vp_dev->vdev, vp_dev->saved_status);
> if (drv && drv->restore)
> return drv->restore(&vp_dev->vdev);
Yes, this looks wrong. Status would typically be DRIVER_OK,
and it needs to be set after vq setup.
It also needs to match current state I think:
e.g. if restore failed we want to set it to FAILED.
So why not do it more or less the way probe does it?
> --
> 1.7.6
^ permalink raw reply
* RE: [RFC net-next PATCH 3/4] ethtool: Add new set commands
From: Rose, Gregory V @ 2011-07-28 22:04 UTC (permalink / raw)
To: Anirban Chakraborty
Cc: Ben Hutchings, netdev, virtualization@lists.linux-foundation.org,
David Miller, Kirsher, Jeffrey T
In-Reply-To: <789BE5A3-93DF-4869-A0AF-180AA86F4AC6@qlogic.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Anirban Chakraborty
> Sent: Thursday, July 28, 2011 3:01 PM
> To: Rose, Gregory V
> Cc: David Miller; netdev; Ben Hutchings; Kirsher, Jeffrey T;
> virtualization@lists.linux-foundation.org
> Subject: Re: [RFC net-next PATCH 3/4] ethtool: Add new set commands
>
>
> On Jul 28, 2011, at 1:38 PM, Rose, Gregory V wrote:
>
> >
> >> From: Anirban Chakraborty [mailto:anirban.chakraborty@qlogic.com]
> >> Sent: Thursday, July 28, 2011 12:04 PM
> >> To: Rose, Gregory V
> >> Cc: David Miller; netdev; Ben Hutchings; Kirsher, Jeffrey T
> >> Subject: Re: [RFC net-next PATCH 3/4] ethtool: Add new set commands
> >>
> >>
> >> If I understood it correctly, you are trying to set/unset spoofing on
> per
> >> eth interface, which could be a PF on the hypervisor or a pci
> passthru-ed
> >> VF in the linux guest. There are other security features that one
> could set
> >> for a port on the VF (lets call it vport), e.g. setting a port VLAN ID
> for
> >> a VF and specifying if the VF (VM) is allowed to send tagged/untagged
> >> packets, setting a vport in port mirroring mode so that the PF can
> monitor
> >> the traffic on a VF, setting a vport in promiscuous mode etc.
> >>
> >> Does it make sense to try to use ip link util to specify all these
> parameters,
> >> since ip link already does the job of setting VF properties and VF
> port
> >> profile?
> >>
> >> Any thoughts?
> >>
> >
> > Sure, that's a possibility too. I was considering ethtool for this
> since MAC addresses and VLANs are fairly specific to Ethernet whereas
> netlink might apply to other types of physical networks. At least that's
> my understanding.
>
> You could specify VF MAC and VLANs using netlink today.
> e.g. ip link set ethX vf # mac, vlan etc.
> Adding spoofing as follows would do it.
> ip link set ethX vf # spoof on|off
>
> Having SR-IOV features scattered among ethtool and ip link may be
> inconvenient for the end users.
> CC-ing virtualization list.
>
> >
> > However, I have no strong feelings about it and if community consensus
> is to use ip link instead then that's fine by me.
> >
> > Of course, patches implementing such would be quite welcome also.
>
> I could take a stab at it at the netlink side, if there is a consensus.
Now that I think about it I'm seeing it more your way. I'll drop the anti-spoofing stuff from my ethtool patches. If you get the time to provide patches to netlink for anti-spoofing then that's great, otherwise I'll do it when I get done with the SR-IOV reconfig stuff.
Thanks,
- Greg
^ permalink raw reply
* Re: [RFC net-next PATCH 3/4] ethtool: Add new set commands
From: Anirban Chakraborty @ 2011-07-28 22:01 UTC (permalink / raw)
To: Rose, Gregory V
Cc: Ben Hutchings, netdev, virtualization@lists.linux-foundation.org,
David Miller, Kirsher, Jeffrey T
In-Reply-To: <43F901BD926A4E43B106BF17856F0755019414D86D@orsmsx508.amr.corp.intel.com>
On Jul 28, 2011, at 1:38 PM, Rose, Gregory V wrote:
>
>> From: Anirban Chakraborty [mailto:anirban.chakraborty@qlogic.com]
>> Sent: Thursday, July 28, 2011 12:04 PM
>> To: Rose, Gregory V
>> Cc: David Miller; netdev; Ben Hutchings; Kirsher, Jeffrey T
>> Subject: Re: [RFC net-next PATCH 3/4] ethtool: Add new set commands
>>
>>
>> On Jul 28, 2011, at 8:51 AM, Rose, Gregory V wrote:
>>
>>
>> -----Original Message-----
>> From: David Miller [mailto:davem@davemloft.net]
>> Sent: Wednesday, July 27, 2011 10:28 PM
>> To: Rose, Gregory V
>> Cc: netdev@vger.kernel.org; bhutchings@solarflare.com; Kirsher, Jeffrey T
>> Subject: Re: [RFC net-next PATCH 3/4] ethtool: Add new set commands
>>
>> From: Greg Rose <gregory.v.rose@intel.com>
>> Date: Wed, 27 Jul 2011 15:17:59 -0700
>>
>> Add new set commands to configure the number of SR-IOV VFs, the
>> number of VM queues and spoof checking on/off switch.
>>
>> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
>> ---
>>
>> include/linux/ethtool.h | 11 ++++++++++-
>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>> index c6e427a..c4972ba 100644
>> --- a/include/linux/ethtool.h
>> +++ b/include/linux/ethtool.h
>> @@ -36,12 +36,14 @@ struct ethtool_cmd {
>> __u8 mdio_support;
>> __u32 maxtxpkt; /* Tx pkts before generating tx int */
>> __u32 maxrxpkt; /* Rx pkts before generating rx int */
>> + __u32 num_vfs; /* Enable SR-IOV VFs */
>> + __u32 num_vmqs; /* Set number of queues for VMDq */
>>
>> You can't change the layout of this datastructure in this way without
>> breaking every ethtool binary out there.
>>
>> You have to find another place to add these knobs.
>>
>> Perhaps at the end of the ethtool_cmd structure? Something like this:
>>
>> /* This should work for both 32 and 64 bit userland. */
>> struct ethtool_cmd {
>> __u32 cmd;
>> __u32 supported; /* Features this interface supports */
>> __u32 advertising; /* Features this interface advertises */
>> __u16 speed; /* The forced speed (lower bits) in
>> * Mbps. Please use
>> * ethtool_cmd_speed()/_set() to
>> * access it */
>> __u8 duplex; /* Duplex, half or full */
>> __u8 port; /* Which connector port */
>> __u8 phy_address;
>> __u8 transceiver; /* Which transceiver to use */
>> __u8 autoneg; /* Enable or disable autonegotiation */
>> __u8 mdio_support;
>> __u32 maxtxpkt; /* Tx pkts before generating tx int */
>> __u32 maxrxpkt; /* Rx pkts before generating rx int */
>> __u16 speed_hi; /* The forced speed (upper
>> * bits) in Mbps. Please use
>> * ethtool_cmd_speed()/_set() to
>> * access it */
>> __u8 eth_tp_mdix;
>> __u8 spoof_check; /* Enable/Disable anti-spoofing */
>> __u32 lp_advertising; /* Features the link partner advertises */
>> __u32 reserved[2];
>> __u32 num_vfs; /* Enable SR-IOV VFs */
>> __u32 num_vmqs; /* Set number of queues for VMDq */
>> };
>>
>> If I understood it correctly, you are trying to set/unset spoofing on per
>> eth interface, which could be a PF on the hypervisor or a pci passthru-ed
>> VF in the linux guest. There are other security features that one could set
>> for a port on the VF (lets call it vport), e.g. setting a port VLAN ID for
>> a VF and specifying if the VF (VM) is allowed to send tagged/untagged
>> packets, setting a vport in port mirroring mode so that the PF can monitor
>> the traffic on a VF, setting a vport in promiscuous mode etc.
>>
>> Does it make sense to try to use ip link util to specify all these parameters,
>> since ip link already does the job of setting VF properties and VF port
>> profile?
>>
>> Any thoughts?
>>
>
> Sure, that's a possibility too. I was considering ethtool for this since MAC addresses and VLANs are fairly specific to Ethernet whereas netlink might apply to other types of physical networks. At least that's my understanding.
You could specify VF MAC and VLANs using netlink today.
e.g. ip link set ethX vf # mac, vlan etc.
Adding spoofing as follows would do it.
ip link set ethX vf # spoof on|off
Having SR-IOV features scattered among ethtool and ip link may be inconvenient for the end users.
CC-ing virtualization list.
>
> However, I have no strong feelings about it and if community consensus is to use ip link instead then that's fine by me.
>
> Of course, patches implementing such would be quite welcome also.
I could take a stab at it at the netlink side, if there is a consensus.
-Anirban
^ permalink raw reply
* [RFC PATCH 8/8] virtio: net: Add freeze, restore handlers to support S4
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <cover.1311863438.git.amit.shah@redhat.com>
Remove all the vqs on hibernation and re-create them after restoring
from a hibernated image. This keeps networking working across
hibernation.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/net/virtio_net.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0c7321c..c9aaa8f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1117,6 +1117,53 @@ static void __devexit virtnet_remove(struct virtio_device *vdev)
free_netdev(vi->dev);
}
+#ifdef CONFIG_PM
+static int virtnet_freeze(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+
+ cancel_delayed_work_sync(&vi->refill);
+
+ /* Free unused buffers in both send and recv, if any. */
+ free_unused_bufs(vi);
+
+ vdev->config->del_vqs(vi->vdev);
+
+ while (vi->pages)
+ __free_pages(get_a_page(vi, GFP_KERNEL), 0);
+
+ return 0;
+}
+
+static int virtnet_restore(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+ struct virtqueue *vqs[3];
+ vq_callback_t *callbacks[] = { skb_recv_done, skb_xmit_done, NULL};
+ const char *names[] = { "input", "output", "control" };
+ int nvqs, err;
+
+ /* We expect two virtqueues, receive then send,
+ * and optionally control. */
+ nvqs = virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ? 3 : 2;
+
+ err = vdev->config->find_vqs(vdev, nvqs, vqs, callbacks, names);
+ if (err)
+ return err;
+
+ vi->rvq = vqs[0];
+ vi->svq = vqs[1];
+
+ if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ)) {
+ vi->cvq = vqs[2];
+ }
+
+ /* Last of all, set up some receive buffers. */
+ try_fill_recv(vi, GFP_KERNEL);
+ return 0;
+}
+#endif
+
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
@@ -1141,6 +1188,10 @@ static struct virtio_driver virtio_net_driver = {
.probe = virtnet_probe,
.remove = __devexit_p(virtnet_remove),
.config_changed = virtnet_config_changed,
+#ifdef CONFIG_PM
+ .freeze = virtnet_freeze,
+ .restore = virtnet_restore,
+#endif
};
static int __init init(void)
--
1.7.6
^ permalink raw reply related
* [RFC PATCH 7/8] virtio: block: Add freeze, restore handlers to support S4
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <cover.1311863438.git.amit.shah@redhat.com>
Delete the vq on the freeze callback to prepare for hibernation.
Re-create the vq in the restore callback to resume normal function.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/block/virtio_blk.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 079c088..700ad76 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -540,6 +540,26 @@ static void __devexit virtblk_remove(struct virtio_device *vdev)
kfree(vblk);
}
+#ifdef CONFIG_PM
+static int virtblk_freeze(struct virtio_device *vdev)
+{
+ vdev->config->del_vqs(vdev);
+ return 0;
+}
+
+static int virtblk_restore(struct virtio_device *vdev)
+{
+ struct virtio_blk *vblk = vdev->priv;
+ int err = 0;
+
+ vblk->vq = virtio_find_single_vq(vdev, blk_done, "requests");
+ if (IS_ERR(vblk->vq))
+ err = PTR_ERR(vblk->vq);
+
+ return err;
+}
+#endif
+
static const struct virtio_device_id id_table[] = {
{ VIRTIO_ID_BLOCK, VIRTIO_DEV_ANY_ID },
{ 0 },
@@ -565,6 +585,10 @@ static struct virtio_driver __refdata virtio_blk = {
.probe = virtblk_probe,
.remove = __devexit_p(virtblk_remove),
.config_changed = virtblk_config_changed,
+#ifdef CONFIG_PM
+ .freeze = virtblk_freeze,
+ .restore = virtblk_restore,
+#endif
};
static int __init init(void)
--
1.7.6
^ permalink raw reply related
* [RFC PATCH 6/8] virtio: console: Add freeze and restore handlers to support S4
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <cover.1311863438.git.amit.shah@redhat.com>
Remove all vqs and associated buffers in the freeze callback which
prepares us to go into hibernation state. On restore, re-create all the
vqs and populate the input vqs with buffers to get to the pre-hibernate
state.
Note: Any outstanding unconsumed buffers are discarded; which means
there's a possibility of data loss in case the host or the guest didn't
consume any data already present in the vqs. This can be addressed in a
later patch series, perhaps in virtio common code.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 71 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index a7dfd02..9648cea 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -25,6 +25,7 @@
#include <linux/fs.h>
#include <linux/init.h>
#include <linux/list.h>
+#include <linux/pm.h>
#include <linux/poll.h>
#include <linux/sched.h>
#include <linux/slab.h>
@@ -1801,6 +1802,72 @@ static unsigned int features[] = {
VIRTIO_CONSOLE_F_MULTIPORT,
};
+#ifdef CONFIG_PM
+static int virtcons_freeze(struct virtio_device *vdev)
+{
+ struct ports_device *portdev;
+ struct port *port;
+ struct port_buffer *buf;
+ unsigned int len;
+
+ portdev = vdev->priv;
+
+ if (use_multiport(portdev)) {
+ while ((buf = virtqueue_get_buf(portdev->c_ivq, &len)))
+ free_buf(buf);
+
+ while ((buf = virtqueue_detach_unused_buf(portdev->c_ivq)))
+ free_buf(buf);
+ }
+ list_for_each_entry(port, &portdev->ports, list) {
+ /*
+ * We'll ask the host later if the new invocation has
+ * the port opened or closed.
+ */
+ port->host_connected = false;
+ discard_port_data(port);
+ reclaim_consumed_buffers(port);
+
+ while ((buf = virtqueue_detach_unused_buf(port->in_vq)))
+ free_buf(buf);
+ }
+ portdev->vdev->config->del_vqs(portdev->vdev);
+ kfree(portdev->in_vqs);
+ kfree(portdev->out_vqs);
+
+ return 0;
+}
+
+static int virtcons_restore(struct virtio_device *vdev)
+{
+ struct ports_device *portdev;
+ struct port *port;
+ int ret;
+
+ portdev = vdev->priv;
+
+ ret = init_vqs(portdev);
+ if (ret)
+ return ret;
+
+ if (use_multiport(portdev)) {
+ fill_queue(portdev->c_ivq, &portdev->cvq_lock);
+ }
+
+ list_for_each_entry(port, &portdev->ports, list) {
+ port->in_vq = portdev->in_vqs[port->id];
+ port->out_vq = portdev->out_vqs[port->id];
+
+ fill_queue(port->in_vq, &port->inbuf_lock);
+
+ /* XXX: Should this be done in 'complete' instead of 'restore'? */
+ /* Get port open/close status on the host */
+ send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
+ }
+ return 0;
+}
+#endif
+
static struct virtio_driver virtio_console = {
.feature_table = features,
.feature_table_size = ARRAY_SIZE(features),
@@ -1810,6 +1877,10 @@ static struct virtio_driver virtio_console = {
.probe = virtcons_probe,
.remove = virtcons_remove,
.config_changed = config_intr,
+#ifdef CONFIG_PM
+ .freeze = virtcons_freeze,
+ .restore = virtcons_restore,
+#endif
};
static int __init init(void)
--
1.7.6
^ permalink raw reply related
* [RFC PATCH 5/8] virtio: console: Use wait_event_freezable instead of _interruptible
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <cover.1311863438.git.amit.shah@redhat.com>
Get ready to support suspend/resume by using the freezable calls so that
blocking read/write syscalls are handled properly across suspend/resume.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 3c10b10..a7dfd02 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -21,6 +21,7 @@
#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/err.h>
+#include <linux/freezer.h>
#include <linux/fs.h>
#include <linux/init.h>
#include <linux/list.h>
@@ -633,8 +634,8 @@ static ssize_t port_fops_read(struct file *filp, char __user *ubuf,
if (filp->f_flags & O_NONBLOCK)
return -EAGAIN;
- ret = wait_event_interruptible(port->waitqueue,
- !will_read_block(port));
+ ret = wait_event_freezable(port->waitqueue,
+ !will_read_block(port));
if (ret < 0)
return ret;
}
@@ -677,8 +678,8 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
if (nonblock)
return -EAGAIN;
- ret = wait_event_interruptible(port->waitqueue,
- !will_write_block(port));
+ ret = wait_event_freezable(port->waitqueue,
+ !will_write_block(port));
if (ret < 0)
return ret;
}
--
1.7.6
^ permalink raw reply related
* [RFC PATCH 4/8] virtio: console: Ignore port name update request if name already set
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <cover.1311863438.git.amit.shah@redhat.com>
We don't allow port name changes dynamically for a port. So any
requests by the host to change the name are ignored.
Before this patch, if the hypervisor sent a port name while we had one
set already, we would leak memory equivalent to the size of the old
name.
This scenario wasn't expected so far, but with the suspend-resume
support, we'll send the VIRTIO_CONSOLE_PORT_READY message after restore,
which can get us into this situation.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index fb68b12..3c10b10 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1394,6 +1394,13 @@ static void handle_control_message(struct ports_device *portdev,
break;
case VIRTIO_CONSOLE_PORT_NAME:
/*
+ * If we woke up after hibernation, we can get this
+ * again. Skip it in that case.
+ */
+ if (port->name)
+ break;
+
+ /*
* Skip the size of the header and the cpkt to get the size
* of the name that was sent
*/
--
1.7.6
^ permalink raw reply related
* [RFC PATCH 3/8] virtio-pci: save/restore config space across S4
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <cover.1311863438.git.amit.shah@redhat.com>
The virtio config space doesn't get saved across hibernation; we save it
locally and update it after restore.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/virtio/virtio_pci.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 579681f..9c37561 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -56,6 +56,10 @@ struct virtio_pci_device
unsigned msix_vectors;
/* Vectors allocated, excluding per-vq vectors if any */
unsigned msix_used_vectors;
+
+ /* Status saved during hibernate/restore */
+ u8 saved_status;
+
/* Whether we have vector per vq */
bool per_vq_vectors;
};
@@ -713,6 +717,7 @@ static int virtio_pci_freeze(struct device *dev)
drv = container_of(vp_dev->vdev.dev.driver,
struct virtio_driver, driver);
+ vp_dev->saved_status = vp_get_status(&vp_dev->vdev);
if (drv && drv->freeze)
return drv->freeze(&vp_dev->vdev);
@@ -728,6 +733,7 @@ static int virtio_pci_restore(struct device *dev)
drv = container_of(vp_dev->vdev.dev.driver,
struct virtio_driver, driver);
+ vp_set_status(&vp_dev->vdev, vp_dev->saved_status);
if (drv && drv->restore)
return drv->restore(&vp_dev->vdev);
--
1.7.6
^ permalink raw reply related
* [RFC PATCH 2/8] virtio-pci: add PM notification handlers for restore, freeze, thaw, poweroff
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <cover.1311863438.git.amit.shah@redhat.com>
Handle restore and freeze notification from the PM core. Expose these
to individual virtio drivers that can quiesce and resume vq operations.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/virtio/virtio_pci.c | 34 ++++++++++++++++++++++++++++++++++
include/linux/virtio.h | 4 ++++
2 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 3879c3e..579681f 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -704,9 +704,43 @@ static int virtio_pci_resume(struct device *dev)
return 0;
}
+static int virtio_pci_freeze(struct device *dev)
+{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
+ struct virtio_driver *drv;
+
+ drv = container_of(vp_dev->vdev.dev.driver,
+ struct virtio_driver, driver);
+
+ if (drv && drv->freeze)
+ return drv->freeze(&vp_dev->vdev);
+
+ return 0;
+}
+
+static int virtio_pci_restore(struct device *dev)
+{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
+ struct virtio_driver *drv;
+
+ drv = container_of(vp_dev->vdev.dev.driver,
+ struct virtio_driver, driver);
+
+ if (drv && drv->restore)
+ return drv->restore(&vp_dev->vdev);
+
+ return 0;
+}
+
static const struct dev_pm_ops virtio_pci_pm_ops = {
.suspend = virtio_pci_suspend,
.resume = virtio_pci_resume,
+ .freeze = virtio_pci_freeze,
+ .thaw = virtio_pci_restore,
+ .restore = virtio_pci_restore,
+ .poweroff = virtio_pci_suspend,
};
#endif
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7108857..9afb662 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -141,6 +141,10 @@ struct virtio_driver {
int (*probe)(struct virtio_device *dev);
void (*remove)(struct virtio_device *dev);
void (*config_changed)(struct virtio_device *dev);
+#ifdef CONFIG_PM
+ int (*freeze)(struct virtio_device *dev);
+ int (*restore)(struct virtio_device *dev);
+#endif
};
int register_virtio_driver(struct virtio_driver *drv);
--
1.7.6
^ permalink raw reply related
* [RFC PATCH 1/8] virtio: pci: switch to new PM API
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
In-Reply-To: <cover.1311863438.git.amit.shah@redhat.com>
The older PM API doesn't have a way to get notifications on hibernate
events, switch to the newer one that gives us those notifications.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/virtio/virtio_pci.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 4bcc8b8..3879c3e 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -17,6 +17,7 @@
#include <linux/module.h>
#include <linux/list.h>
#include <linux/pci.h>
+#include <linux/pm.h>
#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/virtio.h>
@@ -685,19 +686,28 @@ static void __devexit virtio_pci_remove(struct pci_dev *pci_dev)
}
#ifdef CONFIG_PM
-static int virtio_pci_suspend(struct pci_dev *pci_dev, pm_message_t state)
+static int virtio_pci_suspend(struct device *dev)
{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
+
pci_save_state(pci_dev);
pci_set_power_state(pci_dev, PCI_D3hot);
return 0;
}
-static int virtio_pci_resume(struct pci_dev *pci_dev)
+static int virtio_pci_resume(struct device *dev)
{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
+
pci_restore_state(pci_dev);
pci_set_power_state(pci_dev, PCI_D0);
return 0;
}
+
+static const struct dev_pm_ops virtio_pci_pm_ops = {
+ .suspend = virtio_pci_suspend,
+ .resume = virtio_pci_resume,
+};
#endif
static struct pci_driver virtio_pci_driver = {
@@ -706,8 +716,7 @@ static struct pci_driver virtio_pci_driver = {
.probe = virtio_pci_probe,
.remove = __devexit_p(virtio_pci_remove),
#ifdef CONFIG_PM
- .suspend = virtio_pci_suspend,
- .resume = virtio_pci_resume,
+ .driver.pm = &virtio_pci_pm_ops,
#endif
};
--
1.7.6
^ permalink raw reply related
* [RFC PATCH 0/8] virtio: Support for hibernation (S4)
From: Amit Shah @ 2011-07-28 14:35 UTC (permalink / raw)
To: Virtualization List; +Cc: Amit Shah, Michael S. Tsirkin
Hello,
These patches are an initial attempt at supporting hibernation for
virtio drivers.
The default configuration of event_index=on doesn't work; i.e. restore
from a hibernated image only works if the devices have event_index
support turned off. I have not yet dug into this, but is most likely
due to some state not being sync'ed. This could be related to the
hack that is patch 3.
Each virtio driver has to be modified to add support for the
freeze/restore callbacks to delete and then re-add the vqs just before
and after hibernation.
Testing:
I've not tested S3 (suspend).
For virtio-net, ping continues working across hibernate (lost one
packet in sole testing).
For virtio-blk, disk activity continues working.
For virtio-console, port IO continues working.
Amit Shah (8):
virtio: pci: switch to new PM API
virtio-pci: add PM notification handlers for restore, freeze, thaw,
poweroff
virtio-pci: save/restore config space across S4
virtio: console: Ignore port name update request if name already set
virtio: console: Use wait_event_freezable instead of _interruptible
virtio: console: Add freeze and restore handlers to support S4
virtio: block: Add freeze, restore handlers to support S4
virtio: net: Add freeze, restore handlers to support S4
drivers/block/virtio_blk.c | 24 +++++++++++
drivers/char/virtio_console.c | 87 +++++++++++++++++++++++++++++++++++++++--
drivers/net/virtio_net.c | 51 ++++++++++++++++++++++++
drivers/virtio/virtio_pci.c | 57 +++++++++++++++++++++++++--
include/linux/virtio.h | 4 ++
5 files changed, 215 insertions(+), 8 deletions(-)
--
1.7.6
^ permalink raw reply
* Re: [PATCH 5/5] x86-64: Add user_64bit_mode paravirt op
From: Andrew Lutomirski @ 2011-07-27 17:45 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: xen-devel, Konrad Rzeszutek Wilk, x86, Linux Kernel Mailing List,
virtualization, keir.xen
In-Reply-To: <4E3049BA.7060907@goop.org>
On Wed, Jul 27, 2011 at 1:24 PM, Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> On 07/26/2011 08:20 PM, Andy Lutomirski wrote:
>> Three places in the kernel assume that the only long mode CPL 3
>> selector is __USER_CS. This is not true on Xen -- Xen's sysretq
>> changes cs to the magic value 0xe033.
>>
>> Two of the places are corner cases, but as of "x86-64: Improve
>> vsyscall emulation CS and RIP handling"
>> (c9712944b2a12373cb6ff8059afcfb7e826a6c54), vsyscalls will segfault
>> if called with Xen's extra CS selector. This causes a panic when
>> older init builds die.
>>
>> It seems impossible to make Xen use __USER_CS reliably without
>> taking a performance hit on every system call, so this fixes the
>> tests instead with a new paravirt op. It's a little ugly because
>> ptrace.h can't include paravirt.h.
>>
>> Signed-off-by: Andy Lutomirski <luto@mit.edu>
>> Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>> arch/x86/include/asm/desc.h | 4 ++--
>> arch/x86/include/asm/paravirt_types.h | 6 ++++++
>> arch/x86/include/asm/ptrace.h | 19 +++++++++++++++++++
>> arch/x86/kernel/paravirt.c | 4 ++++
>> arch/x86/kernel/step.c | 2 +-
>> arch/x86/kernel/vsyscall_64.c | 6 +-----
>> arch/x86/mm/fault.c | 2 +-
>> arch/x86/xen/enlighten.c | 1 +
>> 8 files changed, 35 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
>> index 7b439d9..41935fa 100644
>> --- a/arch/x86/include/asm/desc.h
>> +++ b/arch/x86/include/asm/desc.h
>> @@ -27,8 +27,8 @@ static inline void fill_ldt(struct desc_struct *desc, const struct user_desc *in
>>
>> desc->base2 = (info->base_addr & 0xff000000) >> 24;
>> /*
>> - * Don't allow setting of the lm bit. It is useless anyway
>> - * because 64bit system calls require __USER_CS:
>> + * Don't allow setting of the lm bit. It would confuse
>> + * user_64bit_mode and would get overridden by sysret anyway.
>> */
>> desc->l = 0;
>> }
>> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
>> index 2c76521..8e8b9a4 100644
>> --- a/arch/x86/include/asm/paravirt_types.h
>> +++ b/arch/x86/include/asm/paravirt_types.h
>> @@ -41,6 +41,7 @@
>>
>> #include <asm/desc_defs.h>
>> #include <asm/kmap_types.h>
>> +#include <asm/pgtable_types.h>
>>
>> struct page;
>> struct thread_struct;
>> @@ -63,6 +64,11 @@ struct paravirt_callee_save {
>> struct pv_info {
>> unsigned int kernel_rpl;
>> int shared_kernel_pmd;
>> +
>> +#ifdef CONFIG_X86_64
>> + u16 extra_user_64bit_cs; /* __USER_CS if none */
>> +#endif
>> +
>> int paravirt_enabled;
>> const char *name;
>> };
>> diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
>> index 94e7618..3566454 100644
>> --- a/arch/x86/include/asm/ptrace.h
>> +++ b/arch/x86/include/asm/ptrace.h
>> @@ -131,6 +131,9 @@ struct pt_regs {
>> #ifdef __KERNEL__
>>
>> #include <linux/init.h>
>> +#ifdef CONFIG_PARAVIRT
>> +#include <asm/paravirt_types.h>
>> +#endif
>>
>> struct cpuinfo_x86;
>> struct task_struct;
>> @@ -187,6 +190,22 @@ static inline int v8086_mode(struct pt_regs *regs)
>> #endif
>> }
>>
>> +#ifdef CONFIG_X86_64
>> +static inline bool user_64bit_mode(struct pt_regs *regs)
>> +{
>> +#ifndef CONFIG_PARAVIRT
>> + /*
>> + * On non-paravirt systems, this is the only long mode CPL 3
>> + * selector. We do not allow long mode selectors in the LDT.
>> + */
>> + return regs->cs == __USER_CS;
>> +#else
>> + /* Headers are too twisted for this to go in paravirt.h. */
>> + return regs->cs == __USER_CS || regs->cs == pv_info.extra_user_64bit_cs;
>
> Is this necessary because usermode may sometimes be on __USER_CS or
> sometimes on Xen's? Could we just commit to one or the other and make
> it a simple comparison?
Currently (from memory), brand new threads start out on __USER_CS.
Also, there might be software out there that thunks back and forth
between 32-bit and 64-bit mode and hardcodes CS=51 as the 32->64 bit
jump target.
It is said that 32-64 bit thunking is impossible, but this is
empirically untrue -- I've done it in the intcc32 vsyscall test I
wrote, and if you remove the actual intcc instruction, it will survive
the thunk in both directions. My code didn't hardcode the assumption.
>
> What if __USER_CS were a variable?
That sounds a little evil :) It will also make the FIXUP_TOP_OF_STACK
macro a little uglier than it is.
But maybe it's not so bad. We could even remove the legacy 64-bit
selector, and presumably everything would still work.
What do you think? I'll benchmark removing VCGF_in_syscall later tonight.
--Andy
>
> J
>> +#endif
>> +}
>> +#endif
>> +
>> /*
>> * X86_32 CPUs don't save ss and esp if the CPU is already in kernel mode
>> * when it traps. The previous stack will be directly underneath the saved
>> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>> index 613a793..d90272e 100644
>> --- a/arch/x86/kernel/paravirt.c
>> +++ b/arch/x86/kernel/paravirt.c
>> @@ -307,6 +307,10 @@ struct pv_info pv_info = {
>> .paravirt_enabled = 0,
>> .kernel_rpl = 0,
>> .shared_kernel_pmd = 1, /* Only used when CONFIG_X86_PAE is set */
>> +
>> +#ifdef CONFIG_X86_64
>> + .extra_user_64bit_cs = __USER_CS,
>> +#endif
>> };
>>
>> struct pv_init_ops pv_init_ops = {
>> diff --git a/arch/x86/kernel/step.c b/arch/x86/kernel/step.c
>> index 7977f0c..c346d11 100644
>> --- a/arch/x86/kernel/step.c
>> +++ b/arch/x86/kernel/step.c
>> @@ -74,7 +74,7 @@ static int is_setting_trap_flag(struct task_struct *child, struct pt_regs *regs)
>>
>> #ifdef CONFIG_X86_64
>> case 0x40 ... 0x4f:
>> - if (regs->cs != __USER_CS)
>> + if (!user_64bit_mode(regs))
>> /* 32-bit mode: register increment */
>> return 0;
>> /* 64-bit mode: REX prefix */
>> diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c
>> index dda7dff..1725930 100644
>> --- a/arch/x86/kernel/vsyscall_64.c
>> +++ b/arch/x86/kernel/vsyscall_64.c
>> @@ -127,11 +127,7 @@ void dotraplinkage do_emulate_vsyscall(struct pt_regs *regs, long error_code)
>>
>> local_irq_enable();
>>
>> - /*
>> - * Real 64-bit user mode code has cs == __USER_CS. Anything else
>> - * is bogus.
>> - */
>> - if (regs->cs != __USER_CS) {
>> + if (!user_64bit_mode(regs)) {
>> /*
>> * If we trapped from kernel mode, we might as well OOPS now
>> * instead of returning to some random address and OOPSing
>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
>> index 4d09df0..decd51a 100644
>> --- a/arch/x86/mm/fault.c
>> +++ b/arch/x86/mm/fault.c
>> @@ -105,7 +105,7 @@ check_prefetch_opcode(struct pt_regs *regs, unsigned char *instr,
>> * but for now it's good enough to assume that long
>> * mode only uses well known segments or kernel.
>> */
>> - return (!user_mode(regs)) || (regs->cs == __USER_CS);
>> + return (!user_mode(regs) || user_64bit_mode(regs));
>> #endif
>> case 0x60:
>> /* 0x64 thru 0x67 are valid prefixes in all modes. */
>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> index 974a528..a9c710a 100644
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -950,6 +950,7 @@ static unsigned xen_patch(u8 type, u16 clobbers, void *insnbuf,
>> static const struct pv_info xen_info __initconst = {
>> .paravirt_enabled = 1,
>> .shared_kernel_pmd = 0,
>> + .extra_user_64bit_cs = FLAT_USER_CS64,
>>
>> .name = "Xen",
>> };
>
>
^ permalink raw reply
* Re: [PATCH 5/5] x86-64: Add user_64bit_mode paravirt op
From: Jeremy Fitzhardinge @ 2011-07-27 17:24 UTC (permalink / raw)
To: Andy Lutomirski
Cc: xen-devel, Konrad Rzeszutek Wilk, x86, Linux Kernel Mailing List,
virtualization, keir.xen
In-Reply-To: <4f4f2b2fdd92eff234f833125732b82a039203e2.1311736366.git.luto@mit.edu>
On 07/26/2011 08:20 PM, Andy Lutomirski wrote:
> Three places in the kernel assume that the only long mode CPL 3
> selector is __USER_CS. This is not true on Xen -- Xen's sysretq
> changes cs to the magic value 0xe033.
>
> Two of the places are corner cases, but as of "x86-64: Improve
> vsyscall emulation CS and RIP handling"
> (c9712944b2a12373cb6ff8059afcfb7e826a6c54), vsyscalls will segfault
> if called with Xen's extra CS selector. This causes a panic when
> older init builds die.
>
> It seems impossible to make Xen use __USER_CS reliably without
> taking a performance hit on every system call, so this fixes the
> tests instead with a new paravirt op. It's a little ugly because
> ptrace.h can't include paravirt.h.
>
> Signed-off-by: Andy Lutomirski <luto@mit.edu>
> Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> arch/x86/include/asm/desc.h | 4 ++--
> arch/x86/include/asm/paravirt_types.h | 6 ++++++
> arch/x86/include/asm/ptrace.h | 19 +++++++++++++++++++
> arch/x86/kernel/paravirt.c | 4 ++++
> arch/x86/kernel/step.c | 2 +-
> arch/x86/kernel/vsyscall_64.c | 6 +-----
> arch/x86/mm/fault.c | 2 +-
> arch/x86/xen/enlighten.c | 1 +
> 8 files changed, 35 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
> index 7b439d9..41935fa 100644
> --- a/arch/x86/include/asm/desc.h
> +++ b/arch/x86/include/asm/desc.h
> @@ -27,8 +27,8 @@ static inline void fill_ldt(struct desc_struct *desc, const struct user_desc *in
>
> desc->base2 = (info->base_addr & 0xff000000) >> 24;
> /*
> - * Don't allow setting of the lm bit. It is useless anyway
> - * because 64bit system calls require __USER_CS:
> + * Don't allow setting of the lm bit. It would confuse
> + * user_64bit_mode and would get overridden by sysret anyway.
> */
> desc->l = 0;
> }
> diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
> index 2c76521..8e8b9a4 100644
> --- a/arch/x86/include/asm/paravirt_types.h
> +++ b/arch/x86/include/asm/paravirt_types.h
> @@ -41,6 +41,7 @@
>
> #include <asm/desc_defs.h>
> #include <asm/kmap_types.h>
> +#include <asm/pgtable_types.h>
>
> struct page;
> struct thread_struct;
> @@ -63,6 +64,11 @@ struct paravirt_callee_save {
> struct pv_info {
> unsigned int kernel_rpl;
> int shared_kernel_pmd;
> +
> +#ifdef CONFIG_X86_64
> + u16 extra_user_64bit_cs; /* __USER_CS if none */
> +#endif
> +
> int paravirt_enabled;
> const char *name;
> };
> diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
> index 94e7618..3566454 100644
> --- a/arch/x86/include/asm/ptrace.h
> +++ b/arch/x86/include/asm/ptrace.h
> @@ -131,6 +131,9 @@ struct pt_regs {
> #ifdef __KERNEL__
>
> #include <linux/init.h>
> +#ifdef CONFIG_PARAVIRT
> +#include <asm/paravirt_types.h>
> +#endif
>
> struct cpuinfo_x86;
> struct task_struct;
> @@ -187,6 +190,22 @@ static inline int v8086_mode(struct pt_regs *regs)
> #endif
> }
>
> +#ifdef CONFIG_X86_64
> +static inline bool user_64bit_mode(struct pt_regs *regs)
> +{
> +#ifndef CONFIG_PARAVIRT
> + /*
> + * On non-paravirt systems, this is the only long mode CPL 3
> + * selector. We do not allow long mode selectors in the LDT.
> + */
> + return regs->cs == __USER_CS;
> +#else
> + /* Headers are too twisted for this to go in paravirt.h. */
> + return regs->cs == __USER_CS || regs->cs == pv_info.extra_user_64bit_cs;
Is this necessary because usermode may sometimes be on __USER_CS or
sometimes on Xen's? Could we just commit to one or the other and make
it a simple comparison?
What if __USER_CS were a variable?
J
> +#endif
> +}
> +#endif
> +
> /*
> * X86_32 CPUs don't save ss and esp if the CPU is already in kernel mode
> * when it traps. The previous stack will be directly underneath the saved
> diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
> index 613a793..d90272e 100644
> --- a/arch/x86/kernel/paravirt.c
> +++ b/arch/x86/kernel/paravirt.c
> @@ -307,6 +307,10 @@ struct pv_info pv_info = {
> .paravirt_enabled = 0,
> .kernel_rpl = 0,
> .shared_kernel_pmd = 1, /* Only used when CONFIG_X86_PAE is set */
> +
> +#ifdef CONFIG_X86_64
> + .extra_user_64bit_cs = __USER_CS,
> +#endif
> };
>
> struct pv_init_ops pv_init_ops = {
> diff --git a/arch/x86/kernel/step.c b/arch/x86/kernel/step.c
> index 7977f0c..c346d11 100644
> --- a/arch/x86/kernel/step.c
> +++ b/arch/x86/kernel/step.c
> @@ -74,7 +74,7 @@ static int is_setting_trap_flag(struct task_struct *child, struct pt_regs *regs)
>
> #ifdef CONFIG_X86_64
> case 0x40 ... 0x4f:
> - if (regs->cs != __USER_CS)
> + if (!user_64bit_mode(regs))
> /* 32-bit mode: register increment */
> return 0;
> /* 64-bit mode: REX prefix */
> diff --git a/arch/x86/kernel/vsyscall_64.c b/arch/x86/kernel/vsyscall_64.c
> index dda7dff..1725930 100644
> --- a/arch/x86/kernel/vsyscall_64.c
> +++ b/arch/x86/kernel/vsyscall_64.c
> @@ -127,11 +127,7 @@ void dotraplinkage do_emulate_vsyscall(struct pt_regs *regs, long error_code)
>
> local_irq_enable();
>
> - /*
> - * Real 64-bit user mode code has cs == __USER_CS. Anything else
> - * is bogus.
> - */
> - if (regs->cs != __USER_CS) {
> + if (!user_64bit_mode(regs)) {
> /*
> * If we trapped from kernel mode, we might as well OOPS now
> * instead of returning to some random address and OOPSing
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index 4d09df0..decd51a 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -105,7 +105,7 @@ check_prefetch_opcode(struct pt_regs *regs, unsigned char *instr,
> * but for now it's good enough to assume that long
> * mode only uses well known segments or kernel.
> */
> - return (!user_mode(regs)) || (regs->cs == __USER_CS);
> + return (!user_mode(regs) || user_64bit_mode(regs));
> #endif
> case 0x60:
> /* 0x64 thru 0x67 are valid prefixes in all modes. */
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 974a528..a9c710a 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -950,6 +950,7 @@ static unsigned xen_patch(u8 type, u16 clobbers, void *insnbuf,
> static const struct pv_info xen_info __initconst = {
> .paravirt_enabled = 1,
> .shared_kernel_pmd = 0,
> + .extra_user_64bit_cs = FLAT_USER_CS64,
>
> .name = "Xen",
> };
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH 0/5] Collected vdso/vsyscall fixes for 3.1
From: Andrew Lutomirski @ 2011-07-27 17:05 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, x86, Linux Kernel Mailing List, virtualization,
keir.xen
In-Reply-To: <20110727165844.GA8064@dumpdata.com>
On Wed, Jul 27, 2011 at 12:58 PM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
>> >> $ test_vsyscall test
>> >> Testing gettimeofday...
>> >> vDSO offset = 0.000001s
>> >> vsyscall offset = 0.000001s
>> >>
>> >> Testing time...
>> >> vDSO offset = 0
>> >> vsyscall offset = 0
>> >> Testing getcpu...
>> >> ok! cpu=6 node=0
>
>> I bet if you pull a new copy or remove -mavx from Makefile it will
>> work. I got a grossly hacked-up Xen domU booted and everything seems
>> to work.
>
> It did. Both Dom0 and DomU work on AMD and Intel.
>
> In regards to the last pv-ops patch - is there no better way? The reason I am asking
> is the pv-ops hook is just a bandaid for the problem. Is the Xen syscall suppose to
> be doingsomething extra with the stack perhaps?
>
The Xen code in question is:
restore_all_guest:
ASSERT_INTERRUPTS_DISABLED
RESTORE_ALL
testw $TRAP_syscall,4(%rsp)
jz iret_exit_to_guest
addq $8,%rsp
popq %rcx # RIP
popq %r11 # CS
cmpw $FLAT_USER_CS32,%r11
popq %r11 # RFLAGS
popq %rsp # RSP
je 1f
sysretq
1: sysretl
So with VCGF_in_syscall set, the ireq hypercall will return via
sysretq if the saved CS is __USER_CS or FLAT_USER_CS64. This is
faster than iretq.
The hypervisor doesn't allow the guest OS to override the values in
MSR_STAR, so FLAT_USER_CS64 gets returned to userspace. And sysretq
is probably much faster than iretq, so unsetting VCGF_in_syscall is
probably a bad idea.
The ideal solution would be to allow the kernel to change MSR_STAR,
but this would require changing the hypervisor and the kernel.
--Andy
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH 0/5] Collected vdso/vsyscall fixes for 3.1
From: Konrad Rzeszutek Wilk @ 2011-07-27 16:58 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: xen-devel, x86, Linux Kernel Mailing List, virtualization,
keir.xen
In-Reply-To: <CAObL_7FncQ7DdNf2igFZW=ZzPFQLO3Vvp1c_y5EajA_KTt8iOg@mail.gmail.com>
> >> $ test_vsyscall test
> >> Testing gettimeofday...
> >> vDSO offset = 0.000001s
> >> vsyscall offset = 0.000001s
> >>
> >> Testing time...
> >> vDSO offset = 0
> >> vsyscall offset = 0
> >> Testing getcpu...
> >> ok! cpu=6 node=0
> I bet if you pull a new copy or remove -mavx from Makefile it will
> work. I got a grossly hacked-up Xen domU booted and everything seems
> to work.
It did. Both Dom0 and DomU work on AMD and Intel.
In regards to the last pv-ops patch - is there no better way? The reason I am asking
is the pv-ops hook is just a bandaid for the problem. Is the Xen syscall suppose to
be doingsomething extra with the stack perhaps?
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH 0/5] Collected vdso/vsyscall fixes for 3.1
From: Konrad Rzeszutek Wilk @ 2011-07-27 16:29 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: xen-devel, x86, Linux Kernel Mailing List, virtualization,
keir.xen
In-Reply-To: <CAObL_7FncQ7DdNf2igFZW=ZzPFQLO3Vvp1c_y5EajA_KTt8iOg@mail.gmail.com>
> 400c8d: c4 e1 f3 2a c8 vcvtsi2sd %rax,%xmm1,%xmm1
>
> which is unlikely to work on AMD unless you're the lucky owner of a
> prerelease Bulldozer chip. I
>
> I bet if you pull a new copy or remove -mavx from Makefile it will
> work. I got a grossly hacked-up Xen domU booted and everything seems
> to work.
Ok.
>
> (Testing native kernels is really fun with qemu-kvm -kernel <image>
> -initrd <my silly initramfs>. But Xen doesn't seem to support that.)
You mean running Xen within QEMU? It does, but you need to use a multiboot
loader and QEMU does not seem to support that. You can create an
.iso image with isolinux that loads mboot.c32 for that.
If you want to use qemu like that in Xen environment you can do it too.
Just need the upstream version and use -M xenpv
^ permalink raw reply
* Re: [PATCH 0/5] Collected vdso/vsyscall fixes for 3.1
From: Andrew Lutomirski @ 2011-07-27 16:15 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, x86, Linux Kernel Mailing List, virtualization,
keir.xen
In-Reply-To: <20110727154316.GA16909@dumpdata.com>
On Wed, Jul 27, 2011 at 11:43 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> On Wed, Jul 27, 2011 at 11:34:21AM -0400, Andrew Lutomirski wrote:
>> On Wed, Jul 27, 2011 at 11:30 AM, Konrad Rzeszutek Wilk
>> <konrad.wilk@oracle.com> wrote:
>> >> > Anyhow, removed the benchmark code and ran it on 64-bit:
>> >> >
>> >> > sh-4.1# /test_vsyscall test
>> >> > Testing gettimeofday...
>> >> > [ 109.552261] test_vsyscall[2462] trap invalid opcode ip:400c8d sp:7fff84fab470 error:0 in test_vsyscall[400000+2000]
>> >> > Illegal instruction
>> >> > sh-4.1# /test_vsyscall intcc
>> >> > About to execute int 0xcc from RIP = 400959
>> >> > [ 114.137150] test_vsyscall[2463] illegal int 0xcc (exploit attempt?) ip:400959 cs:e033 sp:7fff8b328310 ax:2c si:0 di:7fff8b3280f0
>> >> > Caught SIGSEGV: Segmentation fault (Signal sent by the kernel [(nil)])RIP = 400959
>> >> >
>> >> > [This is on git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git #testing, which
>> >> > has todays linus/master and your patchset]
>> >> >
>> >>
>> >> I'll set up Xen. Something's clearly still buggy.
>> >
>> > You sure? This is what I get when I boot baremetal:
>> >
>> > sh-4.1#
>> > sh-4.1# xen-detect
>> > Not running on Xen.
>> > sh-4.1# /test_vsyscall test
>> > Testing gettimeo[ 84.442819] test_vsyscall[3175] trap invalid opcode ip:400c8d sp:7fffa8a72dc0 error:0fday...
>> > in test_vsyscall[400000+2000]
>>
>> $ test_vsyscall test
>> Testing gettimeofday...
>> vDSO offset = 0.000001s
>> vsyscall offset = 0.000001s
>>
>> Testing time...
>> vDSO offset = 0
>> vsyscall offset = 0
>> Testing getcpu...
>> ok! cpu=6 node=0
>>
>> Can you send me your test_vsyscall binary so I can disassemble it?
>
> Here it is (also including source since I uncommented parts of it).
>
> One extra thing - I've been using AMD machines for this - I hadn't
> tried this on an Intel box.
>
Whoops! The offending instruction is:
400c8d: c4 e1 f3 2a c8 vcvtsi2sd %rax,%xmm1,%xmm1
which is unlikely to work on AMD unless you're the lucky owner of a
prerelease Bulldozer chip. I
I bet if you pull a new copy or remove -mavx from Makefile it will
work. I got a grossly hacked-up Xen domU booted and everything seems
to work.
(Testing native kernels is really fun with qemu-kvm -kernel <image>
-initrd <my silly initramfs>. But Xen doesn't seem to support that.)
--Andy
^ permalink raw reply
* Re: [PATCH 0/5] Collected vdso/vsyscall fixes for 3.1
From: Konrad Rzeszutek Wilk @ 2011-07-27 15:43 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: xen-devel, x86, Linux Kernel Mailing List, virtualization,
keir.xen
In-Reply-To: <CAObL_7GWGx4rYJFLpucEX=ozNpk+5ipyq0=vw16xcGetEXSuGQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1796 bytes --]
On Wed, Jul 27, 2011 at 11:34:21AM -0400, Andrew Lutomirski wrote:
> On Wed, Jul 27, 2011 at 11:30 AM, Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com> wrote:
> >> > Anyhow, removed the benchmark code and ran it on 64-bit:
> >> >
> >> > sh-4.1# /test_vsyscall test
> >> > Testing gettimeofday...
> >> > [ 109.552261] test_vsyscall[2462] trap invalid opcode ip:400c8d sp:7fff84fab470 error:0 in test_vsyscall[400000+2000]
> >> > Illegal instruction
> >> > sh-4.1# /test_vsyscall intcc
> >> > About to execute int 0xcc from RIP = 400959
> >> > [ 114.137150] test_vsyscall[2463] illegal int 0xcc (exploit attempt?) ip:400959 cs:e033 sp:7fff8b328310 ax:2c si:0 di:7fff8b3280f0
> >> > Caught SIGSEGV: Segmentation fault (Signal sent by the kernel [(nil)])RIP = 400959
> >> >
> >> > [This is on git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git #testing, which
> >> > has todays linus/master and your patchset]
> >> >
> >>
> >> I'll set up Xen. Something's clearly still buggy.
> >
> > You sure? This is what I get when I boot baremetal:
> >
> > sh-4.1#
> > sh-4.1# xen-detect
> > Not running on Xen.
> > sh-4.1# /test_vsyscall test
> > Testing gettimeo[ 84.442819] test_vsyscall[3175] trap invalid opcode ip:400c8d sp:7fffa8a72dc0 error:0fday...
> > in test_vsyscall[400000+2000]
>
> $ test_vsyscall test
> Testing gettimeofday...
> vDSO offset = 0.000001s
> vsyscall offset = 0.000001s
>
> Testing time...
> vDSO offset = 0
> vsyscall offset = 0
> Testing getcpu...
> ok! cpu=6 node=0
>
> Can you send me your test_vsyscall binary so I can disassemble it?
Here it is (also including source since I uncommented parts of it).
One extra thing - I've been using AMD machines for this - I hadn't
tried this on an Intel box.
[-- Attachment #2: test_vsyscall --]
[-- Type: application/octet-stream, Size: 27973 bytes --]
[-- Attachment #3: test_vsyscall.cc --]
[-- Type: text/x-c++src, Size: 9658 bytes --]
#define _POSIX_SOURCE
#include <stdio.h>
#include <sys/time.h>
#include <time.h>
#include <stdlib.h>
#include <sys/syscall.h>
#include <unistd.h>
#include <dlfcn.h>
#include <string.h>
#include <inttypes.h>
#include <signal.h>
#include <sys/ucontext.h>
#include <asm/ldt.h>
#include <errno.h>
static inline int modify_ldt(int mode, void *ptr, unsigned long size)
{
int ret = syscall(__NR_modify_ldt, mode, ptr, size);
if (ret != 0)
errno = -ret;
return (ret == 0 ? 0 : -1);
}
/* vsyscalls and vDSO */
typedef long (*gtod_t)(struct timeval *tv, struct timezone *tz);
const gtod_t vgtod = (gtod_t)0xffffffffff600000;
gtod_t vdso_gtod;
typedef long (*time_func_t)(time_t *t);
const time_func_t vtime = (time_func_t)0xffffffffff600400;
time_func_t vdso_time;
typedef long (*getcpu_t)(unsigned *, unsigned *, struct getcpu_cache*);
const getcpu_t vgetcpu = (getcpu_t)0xffffffffff600800;
getcpu_t vdso_getcpu;
void init_vdso()
{
void *vdso = dlopen("linux-vdso.so.1", RTLD_LAZY | RTLD_LOCAL | RTLD_NOLOAD);
if (!vdso) {
printf("Warning: failed to find vDSO\n");
return;
}
vdso_gtod = (gtod_t)dlsym(vdso, "gettimeofday");
if (!vdso_gtod)
printf("Warning: failed to find gettimeofday in vDSO\n");
vdso_time = (time_func_t)dlsym(vdso, "time");
if (!vdso_time)
printf("Warning: failed to find time in vDSO\n");
vdso_getcpu = (getcpu_t)dlsym(vdso, "getcpu");
if (!vdso_getcpu)
printf("Warning: failed to find getcpu in vDSO\n");
}
/* syscalls */
static inline long sys_gtod(struct timeval *tv, struct timezone *tz)
{
return syscall(__NR_gettimeofday, tv, tz);
}
static inline long sys_time(time_t *t)
{
return syscall(__NR_time, t);
}
/* There is no sys_getcpu. */
static void segv(int sig, siginfo_t *info, void *ctx_void)
{
psiginfo(info, "Caught SIGSEGV");
ucontext_t *ctx = (ucontext_t*)ctx_void;
printf("RIP = %lx\n", ctx->uc_mcontext.gregs[REG_RIP]);
exit(1);
}
#if 0
/* benchmark helper */
template<typename Func> void benchmark(const char *desc, Func f)
{
struct timespec start, end;
long loops = 0;
printf("Benchmarking %s ... ", desc);
fflush(stdout);
if (clock_gettime(CLOCK_MONOTONIC, &start)) {
perror("clock_gettime");
exit(1);
}
while(true)
{
long loops_now = 1000;
for(int i = 0; i < loops_now; i++)
f();
loops += loops_now;
if (clock_gettime(CLOCK_MONOTONIC, &end)) {
perror("clock_gettime");
exit(1);
}
unsigned long long duration = (end.tv_nsec - start.tv_nsec) +
1000000000ULL * (end.tv_sec - start.tv_sec);
if (duration < 500000000ULL)
continue;
printf("%9ld loops in %.5fs = %7.2f nsec / loop\n",
loops, float(duration) * 1e-9,
float(duration) / loops);
break;
}
}
#endif
static double tv_diff(const struct timeval &a, const struct timeval &b)
{
return double(a.tv_sec - b.tv_sec) +
double((int)a.tv_usec - (int)b.tv_usec) * 1e-6;
}
int test(int argc, char **argv)
{
printf("Testing gettimeofday...\n");
struct timeval tv_sys, tv_vdso, tv_vsys;
struct timezone tz_sys, tz_vdso, tz_vsys;
int ret_sys = sys_gtod(&tv_sys, &tz_sys);
int ret_vdso = -1;
if (vdso_gtod)
ret_vdso = vdso_gtod(&tv_vdso, &tz_vdso);
int ret_vsys = vgtod(&tv_vsys, &tz_vsys);
if (ret_sys) {
printf(" syscall failed\n");
} else {
if (ret_vdso == 0) {
if (tz_sys.tz_minuteswest != tz_vdso.tz_minuteswest || tz_sys.tz_dsttime != tz_vdso.tz_dsttime)
printf(" vDSO tz mismatch\n");
else
printf(" vDSO offset = %.6fs\n", tv_diff(tv_vdso, tv_sys));
} else if (vdso_gtod) {
printf(" vDSO failed\n");
}
if (ret_vsys == 0) {
if (tz_sys.tz_minuteswest != tz_vsys.tz_minuteswest || tz_sys.tz_dsttime != tz_vsys.tz_dsttime)
printf(" vsyscall tz mismatch\n");
else
printf(" vsyscall offset = %.6fs\n", tv_diff(tv_vsys, tv_sys));
}
}
printf("\nTesting time...\n");
long t_sys, t_vdso = 0, t_vsys;
long t2_sys = -1, t2_vdso = -1, t2_vsys = -1;
t_sys = sys_time(&t2_sys);
if (vdso_time)
t_vdso = vdso_time(&t2_vdso);
t_vsys = vtime(&t2_vsys);
if (t_sys < 0 || t_sys != t2_sys) {
printf(" syscall failed (ret:%ld output:%ld)\n", t_sys, t2_sys);
} else {
if (vdso_time) {
if (t_vdso < 0 || t_vdso != t2_vdso)
printf(" vDSO failed (ret:%ld output:%ld)\n", t_vdso, t2_vdso);
else
printf(" vDSO offset = %ld\n", t_vdso - t_sys);
}
if (t_vsys < 0 || t_vsys != t2_vsys)
printf(" vsyscall failed (ret:%ld output:%ld)\n", t_vsys, t2_vsys);
else
printf(" vsyscall offset = %ld\n", t_vsys - t_sys);
}
printf("Testing getcpu...\n");
unsigned cpu_vdso, cpu_vsys, node_vdso, node_vsys;
ret_vdso = vdso_getcpu(&cpu_vdso, &node_vdso, 0);
ret_vsys = vgetcpu(&cpu_vsys, &node_vsys, 0);
if (ret_vdso)
printf(" vDSO failed (ret:%ld)\n", (unsigned long)ret_vdso);
if (ret_vsys)
printf(" vsyscall failed (ret:%ld)\n", (unsigned long)ret_vdso);
if (ret_vdso == 0 && ret_vsys == 0) {
if (cpu_vdso != cpu_vsys)
printf(" cpu mismatch (vdso:%u vsyscall:%u)!\n", cpu_vdso, cpu_vsys);
else if (node_vdso != node_vsys)
printf(" node mismatch (vdso:%u vsyscall:%u)!\n", node_vdso, node_vsys);
else
printf(" ok! cpu=%u node=%u\n", cpu_vdso, node_vdso);
}
return 0;
}
int bench(int argc, char **argv)
{
struct timeval tv;
struct timezone tz;
#if 0
benchmark(" syscall gettimeofday", [&]{sys_gtod(&tv, &tz);});
benchmark(" vdso gettimeofday", [&]{vdso_gtod(&tv, &tz);});
benchmark("vsyscall gettimeofday", [&]{vgtod(&tv, &tz);});
printf("\n");
time_t t;
benchmark(" syscall time ", [&]{sys_time(&t);});
if (vdso_time)
benchmark(" vdso time ", [&]{vdso_time(&t);});
benchmark("vsyscall time ", [&]{vtime(&t);});
printf("\n");
unsigned cpu, node;
benchmark(" vdso getcpu ", [&]{vdso_getcpu(&cpu, &node, 0);});
benchmark("vsyscall getcpu ", [&]{vgetcpu(&cpu, &node, 0);});
printf("\n");
benchmark("dummy syscall ", [&]{syscall(0xffffffff);});
#endif
return 0;
}
int call(int argc, char **argv)
{
if (argc != 5) {
printf("Usage: call <addr> <rax> <arg1> <arg2> <arg3>\n");
return 1;
}
unsigned long addr, rax, arg1, arg2, arg3;
char *end;
addr = strtoull(argv[0], &end, 0);
if (*end)
goto bad;
rax = strtoull(argv[1], &end, 0);
if (*end)
goto bad;
arg1 = strtoull(argv[2], &end, 0);
if (*end)
goto bad;
arg2 = strtoull(argv[3], &end, 0);
if (*end)
goto bad;
arg3 = strtoull(argv[4], &end, 0);
if (*end)
goto bad;
unsigned long ret;
asm volatile("call *%[addr]" : "=a" (ret) : [addr] "rm" (addr), "a" (rax),
"D" (arg1), "S" (arg2), "d" (arg3));
printf("Return value = %ld\n", ret);
return 0;
bad:
printf("Bad arg\n");
return 1;
}
int intcc(int argc, char **argv)
{
if (argc != 0) {
printf("Usage: intcc\n");
return 1;
}
extern char intcc_addr;
printf("About to execute int 0xcc from RIP = %lX\n",
(unsigned long)&intcc_addr);
asm volatile ("intcc_addr: int $0xcc");
return 0;
}
struct __attribute__((packed)) farptr {
uint32_t offset;
uint16_t sel;
};
static bool to_farptr(farptr *out, uint16_t sel, void *offset)
{
out->sel = sel;
out->offset = (uint32_t)(unsigned long)offset;
return out->offset == (unsigned long)offset;
}
int intcc32(int argc, char **argv)
{
if (argc != 0) {
printf("Usage: intcc32\n");
return 1;
}
// Install a 32-bit code descriptor
struct user_desc desc;
memset(&desc, 0, sizeof(desc));
desc.entry_number = 0;
desc.base_addr = 0;
desc.limit = 0xFFFFF;
desc.seg_32bit = 1;
desc.contents = MODIFY_LDT_CONTENTS_CODE;
desc.limit_in_pages = 1;
if (modify_ldt(1, &desc, sizeof(desc)) != 0) {
perror("modify_ldt");
return 1;
}
/* Load the initial CS. */
uint16_t initial_cs;
asm ("mov %%cs,%[initial_cs]" : [initial_cs] "=rm" (initial_cs));
printf("Initial CS = 0x%04X (entry %d)\n",
(unsigned)initial_cs, (int)(initial_cs >> 3));
extern char landing_32, landing_64;
/* Set up the pointers. */
static farptr ptr32, ptr64;
if (!to_farptr(&ptr32, 0x4, &landing_32) || !to_farptr(&ptr64, initial_cs, &landing_64)) {
printf("Something's mapped too high\n");
return 1;
}
/* Go for it! */
asm volatile (
"mov %%rsp,%%rsi\n" // Save rsp (avoids truncation).
"ljmpl *(%%eax)\n" // Switch to 32-bit mode.
// 32-bit mode!
// (Well, sort of. DS and ES are 0, so we can't use them.)
".code32\n"
"landing_32:\n"
"\tint $0xcc\n" // Try int 0xcc.
"\tljmpl *%%cs:(%%ecx)\n" // Switch back.
// 64-bit mode again!
".code64\n"
"landing_64:\n"
"\tmov %%rsi,%%rsp"
:
: "a" (&ptr32), "c" (&ptr64)
: "rsi", "cc");
printf("Holy cow! We survived!\n");
return 0;
}
int main(int argc, char **argv)
{
struct sigaction sa_segv;
memset(&sa_segv, 0, sizeof(sa_segv));
sa_segv.sa_sigaction = segv;
sa_segv.sa_flags = SA_SIGINFO;
sigemptyset(&sa_segv.sa_mask);
if (sigaction(SIGSEGV, &sa_segv, 0))
perror("sigaction");
init_vdso();
if (argc < 2) {
printf("Usage: test_vsyscall <command> ...\n"
"command := { test, bench, intcc, call }\n");
return 1;
}
if (!strcmp(argv[1], "test"))
return test(argc - 2, argv + 2);
if (!strcmp(argv[1], "bench"))
return bench(argc - 2, argv + 2);
if (!strcmp(argv[1], "intcc"))
return intcc(argc - 2, argv + 2);
if (!strcmp(argv[1], "intcc32"))
return intcc32(argc - 2, argv + 2);
if (!strcmp(argv[1], "call"))
return call(argc - 2, argv + 2);
printf("Unknown command\n");
return 1;
}
[-- Attachment #4: Type: text/plain, Size: 184 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox