On Fri, Mar 3, 2017 at 11:45 AM, 'Sergiy Kibrik' via OSv Development <
osv-dev@googlegroups.com> wrote:
> Xen platform PCI is x86-specific feature, not currently present on aarch64.
>
> For xenbus to work on both platforms the generic xenbus driver should be
> probed
> directly (under aarch64 in case Xen is detected), but in case of x64
> special PCI
> driver checks for Xen platform PCI device and probes generic xenbus driver
> if such device is found, pretty much like the way it is done in Linux.
>
> Signed-off-by: Sergiy Kibrik <sergiy.kib...@globallogic.com>
> ---
> Makefile | 1 +
> arch/x64/arch-setup.cc | 4 ++--
> drivers/xenfront-xenbus.cc | 33 ++++-----------------------------
> drivers/xenfront-xenbus.hh | 12 +++---------
> drivers/xenfront.hh | 2 --
> drivers/xenplatform-pci.cc | 40 ++++++++++++++++++++++++++++++++++++++++
> drivers/xenplatform-pci.hh | 33 +++++++++++++++++++++++++++++++++
> 7 files changed, 83 insertions(+), 42 deletions(-)
> create mode 100644 drivers/xenplatform-pci.cc
> create mode 100644 drivers/xenplatform-pci.hh
>
> diff --git a/Makefile b/Makefile
> index 6846c5e..3f011fa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -818,6 +818,7 @@ drivers += drivers/ahci.o
> drivers += drivers/ide.o
> drivers += drivers/scsi-common.o
> drivers += drivers/vmw-pvscsi.o
> +drivers += drivers/xenplatform-pci.o
> endif # x64
>
> ifeq ($(arch),aarch64)
> diff --git a/arch/x64/arch-setup.cc b/arch/x64/arch-setup.cc
> index fb5deaa..325c26a 100644
> --- a/arch/x64/arch-setup.cc
> +++ b/arch/x64/arch-setup.cc
> @@ -255,7 +255,7 @@ void arch_init_premain()
> #include "drivers/virtio-net.hh"
> #include "drivers/virtio-assign.hh"
> #include "drivers/virtio-rng.hh"
> -#include "drivers/xenfront-xenbus.hh"
> +#include "drivers/xenplatform-pci.hh"
> #include "drivers/ahci.hh"
> #include "drivers/vmw-pvscsi.hh"
> #include "drivers/vmxnet3.hh"
> @@ -283,7 +283,7 @@ void arch_init_drivers()
> drvman->register_driver(virtio::net::probe);
> }
> drvman->register_driver(virtio::rng::probe);
> - drvman->register_driver(xenfront::xenbus::probe);
> + drvman->register_driver(xenfront::xenplatform_pci::probe);
> drvman->register_driver(ahci::hba::probe);
> drvman->register_driver(vmw::pvscsi::probe);
> drvman->register_driver(vmw::vmxnet3::probe);
> diff --git a/drivers/xenfront-xenbus.cc b/drivers/xenfront-xenbus.cc
> index 3f6ab29..ba14384 100644
> --- a/drivers/xenfront-xenbus.cc
> +++ b/drivers/xenfront-xenbus.cc
> @@ -5,8 +5,6 @@
> * BSD license as described in the LICENSE file in the top-level
> directory.
> */
>
> -#include "msr.hh"
> -#include "xen.hh"
> #include <osv/types.h>
> #include <osv/mmu.hh>
> #include "string.h"
> @@ -46,27 +44,17 @@ namespace xenfront {
>
> xenbus *xenbus::_instance = nullptr;
>
> -xenbus::xenbus(pci::device& pci_dev)
> +xenbus::xenbus()
> : hw_driver()
>
Does xenbus still need to be a "hw_driver" instance, or can just be utility
code called from an actual hw_driver?
> - , _dev(pci_dev)
> {
> - int irqno = pci_dev.get_interrupt_line();
> -
> if (_instance) {
> return;
> } else {
> _instance = this;
> }
>
> - parse_pci_config();
> -
> - _dev.set_bus_master(true);
> _driver_name = std::string("xenfront-xenbus");
>
> - if (!processor::features().xen_vector_callback) {
> - _pgsi.reset(xen::xen_set_callback(irqno));
> - }
> -
> xs_attach(&_xenstore_device);
>
> xs_scanf(XST_NIL, "domid", "", NULL, "%d", &_domid);
> @@ -127,27 +115,14 @@ void
> xenbus::for_each_child(std::function<void(xenfront_driver
> *d)> func)
>
> hw_driver* xenbus::probe(hw_device* dev)
> {
> - if (!processor::features().xen_pci) {
> + if (!is_xen())
> return nullptr;
> - }
> -
> - if (auto pci_dev = dynamic_cast<pci::device*>(dev)) {
> - // dev id is the same for all xen devices?
> - if (pci_dev->get_subsystem_vid() == XEN_VENDOR_ID) {
> - return new xenbus(*pci_dev);
> - }
> - }
> - return nullptr;
> + return new xenbus();
> }
>
If I understand correctly, this function no longer "probes" anything (it
doesn't even use the "dev" parameter). So shouldn't you rename it and
remove its argument, or even delete it altogether (just incorporating its
code into the caller)?
>
> void xenbus::dump_config()
> {
> - _dev.dump_config();
> -}
> -
> -bool xenbus::parse_pci_config()
> -{
> - return true;
> + /*TODO: print type, name and node path */
> }
> };
>
> diff --git a/drivers/xenfront-xenbus.hh b/drivers/xenfront-xenbus.hh
> index 808545a..4a587a4 100644
> --- a/drivers/xenfront-xenbus.hh
> +++ b/drivers/xenfront-xenbus.hh
> @@ -9,22 +9,18 @@
> #define XENFRONT_BUS_DRIVER_H
>
> #include "drivers/xenfront.hh"
> -#include "drivers/pci-device.hh"
> +#include "drivers/device.hh"
> #include <osv/device.h>
> #include <bsd/porting/bus.h>
>
> namespace xenfront {
> -
> class xenbus : public hw_driver {
> public:
>
> - explicit xenbus(pci::device& dev);
> + explicit xenbus();
> static hw_driver* probe(hw_device* dev);
> - pci::device& pci_device() { return _dev; }
> -
> - bool parse_pci_config();
> - void dump_config();
>
> + virtual void dump_config();
> virtual std::string get_name() const { return _driver_name; }
> const std::string &get_node_path() { return _node_path; }
>
> @@ -40,8 +36,6 @@ namespace xenfront {
> private:
> static struct xenbus *_instance;
> void wait_for_devices();
> - pci::device& _dev;
> - std::unique_ptr<gsi_level_interrupt> _pgsi;
> struct device _xenstore_device;
>
> std::vector<xenfront_driver *> _children;
> diff --git a/drivers/xenfront.hh b/drivers/xenfront.hh
> index eaea39e..8f239dc 100644
> --- a/drivers/xenfront.hh
> +++ b/drivers/xenfront.hh
> @@ -18,8 +18,6 @@
> #include <bsd/porting/bus.h>
> #include <xen/interface/io/xenbus.h>
>
> -#define XEN_VENDOR_ID 0x5853
> -
> struct xenbus_device_ivars;
>
> template <typename T>
> diff --git a/drivers/xenplatform-pci.cc b/drivers/xenplatform-pci.cc
> new file mode 100644
> index 0000000..8e5f439
> --- /dev/null
> +++ b/drivers/xenplatform-pci.cc
>
A silly question from someone who doesn't know much about Xen: What's the
difference between "xenplatform", "xenfront", and, I guess, "xenbus"?
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (C) 2017 Sergiy Kibrik <sergiy.kib...@globallogic.com>
> + *
> + * This work is open source software, licensed under the terms of the
> + * BSD license as described in the LICENSE file in the top-level
> directory.
> + */
>
If you copied code from other places to this file, can you please keep
*also* the original Cloudius Systems copyright 2013 (?) line? Not that this
really matters ;-)
> +
> +#include "xen.hh"
> +#include "xenplatform-pci.hh"
> +
> +#define XEN_VENDOR_ID 0x5853
> +#define XEN_DEVICE_ID 0x0001
> +
> +namespace xenfront {
> +
> +hw_driver* xenplatform_pci::probe(hw_device *dev)
> +{
> + if (!processor::features().xen_pci)
> + return nullptr;
> +
> + if (dev->get_id() == hw_device_id(XEN_VENDOR_ID, XEN_DEVICE_ID)) {
> + auto pci_dev = dynamic_cast<pci::device*>(dev);
> + return new xenplatform_pci(*pci_dev);
> + }
> +
> + return nullptr;
> +}
> +
> +xenplatform_pci::xenplatform_pci(pci::device& dev)
> + : hw_driver()
> + , _dev(dev)
> +{
> + _dev.set_bus_master(true);
> + if (!processor::features().xen_vector_callback) {
> + int irqno = _dev.get_interrupt_line();
> + _pgsi.reset(xen::xen_set_callback(irqno));
> + }
> + _xenbus.reset(xenfront::xenbus::probe(NULL));
> +}
> +}
> diff --git a/drivers/xenplatform-pci.hh b/drivers/xenplatform-pci.hh
> new file mode 100644
> index 0000000..e8a28d4
> --- /dev/null
> +++ b/drivers/xenplatform-pci.hh
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright (C) 2017 Sergiy Kibrik <sergiy.kib...@globallogic.com>
> + *
> + * This work is open source software, licensed under the terms of the
> + * BSD license as described in the LICENSE file in the top-level
> directory.
> + */
> +
> +#ifndef XENPLATFORM_PCI_H
> +#define XENPLATFORM_PCI_H
> +
> +#include <drivers/driver.hh>
> +#include <drivers/xenfront-xenbus.hh>
> +
> +namespace xenfront {
> +
> +class xenplatform_pci : public hw_driver {
> +
> +public:
> + static hw_driver* probe(hw_device* dev);
> +
> + explicit xenplatform_pci(pci::device& dev);
> + virtual void dump_config() { _dev.dump_config(); }
> + virtual std::string get_name() const {
> + return std::string("xen-platform-pci");
> + }
> +
> +private:
> + pci::device& _dev;
> + std::unique_ptr<gsi_level_interrupt> _pgsi;
> + std::unique_ptr<hw_driver> _xenbus;
> +};
> +}
> +#endif /* XENPLATFORM_PCI_H */
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
