diff mbox series

Adds virtio_net as the default netcard for mips boston board.

Message ID 1554099897-28606-2-git-send-email-tjin@wavecomp.com
State New
Headers show
Series Adds virtio_net as the default netcard for mips boston board. | expand

Commit Message

Tommy Jin April 1, 2019, 6:26 a.m. UTC
Boston didn't bring up any netcard by default, this is not so convenient for users who are verifying network related functionalities on this board.
As the linux kernel has already supported virtio_net, now add a virtio netcard for boston, to make virtio work properly, please:
1. set virtio net options in linux kernel, saying CONFIG_VIRTIO && CONFIG_VIRTIO_PCI && CONFIG_VIRTIO_NET
2. specify model=virtio when you start boston in qemu.

Signed-off-by: tjin <tjin@wavecomp.com>
---
 hw/mips/boston.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Peter Maydell April 1, 2019, 9:38 a.m. UTC | #1
On Mon, 1 Apr 2019 at 16:23, Tommy Jin <tjin@wavecomp.com> wrote:
>
> Boston didn't bring up any netcard by default, this is not so convenient for users who are verifying network related functionalities on this board.
> As the linux kernel has already supported virtio_net, now add a virtio netcard for boston, to make virtio work properly, please:
> 1. set virtio net options in linux kernel, saying CONFIG_VIRTIO && CONFIG_VIRTIO_PCI && CONFIG_VIRTIO_NET
> 2. specify model=virtio when you start boston in qemu.
>
> Signed-off-by: tjin <tjin@wavecomp.com>
> ---
>  hw/mips/boston.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index cac0e22..5d3239e 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -428,10 +428,10 @@ static void network_init(PCIBus *bus)
>  {
>      PCIDevice *eth;
>
> -    /*Please set CONFIG_VIRTIO && CONFIG_VIRTIO_PCI && CONFIG_VIRTIO_NET
> -        of Linux kernel.*/
> +    /* Please set CONFIG_VIRTIO && CONFIG_VIRTIO_PCI && CONFIG_VIRTIO_NET */
> +    /* of Linux kernel. */

This sort of thing should go in documentation, not as a comment
in the code.

>      eth = pci_create(bus,
> -                                        PCI_DEVFN(0, 1), "virtio-net-pci");
> +                     PCI_DEVFN(0, 1), "virtio-net-pci");


Am I missing something, or is this patch not actually changing
any code? Maybe you made a mistake somehow when creating it?

I'm not sure if we want to create a default network card
for this board -- the user (or more usually the management
layer like libvirt) can create it on the command line, and
if we always create a virtio-net-pci device in the code then
the user has no way to say they *don't* want the device.

Also, this board is a model of a real piece of hardware, and
the real hardware doesn't have a virtio-net device in it.

>
>      qdev_set_nic_properties(&eth->qdev, &nd_table[0]);
>      qdev_init_nofail(&eth->qdev);
> --

thanks
-- PMM
Aleksandar Markovic April 2, 2019, 2:25 a.m. UTC | #2
On Apr 1, 2019 11:39 AM, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>
> On Mon, 1 Apr 2019 at 16:23, Tommy Jin <tjin@wavecomp.com> wrote:
> >
> > Boston didn't bring up any netcard by default, this is not so
convenient for users who are verifying network related functionalities on
this board.
> > As the linux kernel has already supported virtio_net, now add a virtio
netcard for boston, to make virtio work properly, please:
> > 1. set virtio net options in linux kernel, saying CONFIG_VIRTIO &&
CONFIG_VIRTIO_PCI && CONFIG_VIRTIO_NET
> > 2. specify model=virtio when you start boston in qemu.
> >
> > Signed-off-by: tjin <tjin@wavecomp.com>
> > ---
> >  hw/mips/boston.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> > index cac0e22..5d3239e 100644
> > --- a/hw/mips/boston.c
> > +++ b/hw/mips/boston.c
> > @@ -428,10 +428,10 @@ static void network_init(PCIBus *bus)
> >  {
> >      PCIDevice *eth;
> >
> > -    /*Please set CONFIG_VIRTIO && CONFIG_VIRTIO_PCI &&
CONFIG_VIRTIO_NET
> > -        of Linux kernel.*/
> > +    /* Please set CONFIG_VIRTIO && CONFIG_VIRTIO_PCI &&
CONFIG_VIRTIO_NET */
> > +    /* of Linux kernel. */
>
> This sort of thing should go in documentation, not as a comment
> in the code.
>
> >      eth = pci_create(bus,
> > -                                        PCI_DEVFN(0, 1),
"virtio-net-pci");
> > +                     PCI_DEVFN(0, 1), "virtio-net-pci");
>
>
> Am I missing something, or is this patch not actually changing
> any code? Maybe you made a mistake somehow when creating it?
>
> I'm not sure if we want to create a default network card
> for this board -- the user (or more usually the management
> layer like libvirt) can create it on the command line, and
> if we always create a virtio-net-pci device in the code then
> the user has no way to say they *don't* want the device.
>
> Also, this board is a model of a real piece of hardware, and
> the real hardware doesn't have a virtio-net device in it.
>
> >
> >      qdev_set_nic_properties(&eth->qdev, &nd_table[0]);
> >      qdev_init_nofail(&eth->qdev);
> > --
>
> thanks
> -- PMM
>

Just adding Paul Burton, the Boston card emulation maintainer.

Aleksandar
Tommy Jin April 2, 2019, 2:29 a.m. UTC | #3
Hi Peter,


Thank you very much for you comments,I sent a wrong patch out by mistake.



If always creating virtio-net-pci device is not a good idea, is it feasible to make virtio-net-pci as an option for boston without using libvirt? we can only create this device when the user specify "model=virtio".


Actually, Botson board can have an Intel GBE network card in it, compared to mips malta, it also has a pcnet network card created in its machine, is it acceptable to create a pch_gbe network card if the user specify like "model=pch_gbe"?


Best regards,

Tommy
Peter Maydell April 2, 2019, 2:37 a.m. UTC | #4
On Tue, 2 Apr 2019 at 09:29, Tommy Jin <tjin@wavecomp.com> wrote:
> If always creating virtio-net-pci device is not a good idea, is it
> feasible to make virtio-net-pci as an option for boston without
> using libvirt?

It's always an option, just specify it on the command line.

> Actually, Botson board can have an Intel GBE network card in it,
> compared to mips malta, it also has a pcnet network card created
> in its machine, is it acceptable to create a pch_gbe network card
> if the user specify like "model=pch_gbe"?

Again, optional pluggable cards should just be created without
any board code required if the user specifies them on the command line.

In general, the QEMU command line does not aim for "short,
do what the user probably wants without them specifying
many options". It aims for "be flexible, allow the user
to configure what they want". The user-friendly interface
is supposed to be the next level up (libvirt, etc). This does
mean that command lines can get quite long in some cases. That's
unfortunate but I don't think we should be putting workarounds
for that in individual board models.

thanks
-- PMM
Aleksandar Markovic April 2, 2019, 2:43 a.m. UTC | #5
On Apr 2, 2019 4:29 AM, "Tommy Jin" <tjin@wavecomp.com> wrote:
>
> Hi Peter,
>
>
> Thank you very much for you comments,I sent a wrong patch out by mistake.
>
>
>
> If always creating virtio-net-pci device is not a good idea, is it
feasible to make virtio-net-pci as an option for boston without using
libvirt? we can only create this device when the user specify
"model=virtio".
>
>
> Actually, Botson board can have an Intel GBE network card in it, compared
to mips malta, it also has a pcnet network card created in its machine, is
it acceptable to create a pch_gbe network card if the user specify like
"model=pch_gbe"?
>
>
> Best regards,
>
> Tommy
>
>

Hello, Tommy.

Thanks for your efforts in this area. However, your patches will have
greater chances of serious considering and accepting if you follow the
rules of submitting them to this list. There is a page on contributing
called “How to submit a patch“ on QEMU website. Please read it carefully
and follow each step.

Welcome to qemu-devel! I wish you continue to contribute to QEMU many
improvements.

Aleksandar
Tommy Jin April 2, 2019, 3:03 a.m. UTC | #6
Hi Aleksandar,


I understand, Thank you very much for reminding me.


Regards,

Tommy
Tommy Jin April 2, 2019, 7:24 a.m. UTC | #7
Hi Peter,

I'm glad to hear these qemu general rules from you.
Here you raised two options:

  1.  Using a tool like libvirt to add a pluggable network card
  2.  Specify the network card on the command line

For the option2, I think specify the network model in command line is not enough, per my understanding, we still need to create the device using API like pci_create && qdev_set_nic_properties && qdev_init_nofail, otherwise we'll get error prompts like "requested NIC (anonymous, model virtio) was not created (not supported by this machine?)", currently I just add these pieces of code in /hw/mips/boston.c, Do you call these pieces of code board code and what's your suggestion?

You mentioned "optional pluggable cards should just be created without any board code required if the user specifies them on the command line."
I don't quite understand what you mean, if I don't want to use option1, how to create pluggable cards without any board code?

Best regards,
Tommy
Peter Maydell April 2, 2019, 7:35 a.m. UTC | #8
On Tue, 2 Apr 2019 at 14:24, Tommy Jin <tjin@wavecomp.com> wrote:
> For the option2, I think specify the network model in command line is
> not enough, per my understanding, we still need to create the device
> using API like pci_create && qdev_set_nic_properties && qdev_init_nofail,
> otherwise we'll get error prompts like "requested NIC (anonymous, model
> virtio) was not created (not supported by this machine?)", currently I
> just add these pieces of code in /hw/mips/boston.c, Do you call these
> pieces of code board code and what's your suggestion?

If the boston board has PCI support and it is implemented correctly
then you should be able to create a virtio network setup with
command line options like
 -netdev user,id=mynet -device virtio-net-pci,netdev=mynet

(or any other netdev backend, eg tun/tap).

I don't have a boston image/command line to test that with,
but it works for arm boards and should be the same for any
pci capable board model.

thanks
-- PMM
Tommy Jin April 3, 2019, 4:52 a.m. UTC | #9
Hi Peter,


That command line option doesn't work for boston, although I didn't get any obvious error, no Ethernet card appears either.

Where can I get that arm image/command which can work well for you with " -netdev user,id=mynet -device virtio-net-pci,netdev=mynet" ?


Anyway, if that command line option works for arm, I'm going to investigate the reason.


Best regards,

Tommy
Peter Maydell April 3, 2019, 5:03 a.m. UTC | #10
On Wed, 3 Apr 2019 at 11:52, Tommy Jin <tjin@wavecomp.com> wrote:
> That command line option doesn't work for boston, although I didn't get any obvious error, no Ethernet card appears either.

Does the guest kernel find the PCI device (eg appears in kernel bootup
messages, appears in 'lspci' output)? It's worth double-checking that
your guest kernel's config has enabled support for PCI, for the PCI
controller used by the boston board, for virtio-net, etc.

> Where can I get that arm image/command which can work well for you with " -netdev user,id=mynet -device virtio-net-pci,netdev=mynet" ?

https://translatedcode.wordpress.com/2017/07/24/installing-debian-on-qemus-64-bit-arm-virt-board/
has instructions for booting Debian which use that command line.

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index cac0e22..5d3239e 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -428,10 +428,10 @@  static void network_init(PCIBus *bus)
 {
     PCIDevice *eth;
 
-    /*Please set CONFIG_VIRTIO && CONFIG_VIRTIO_PCI && CONFIG_VIRTIO_NET
-        of Linux kernel.*/
+    /* Please set CONFIG_VIRTIO && CONFIG_VIRTIO_PCI && CONFIG_VIRTIO_NET */
+    /* of Linux kernel. */
     eth = pci_create(bus,
-					 PCI_DEVFN(0, 1), "virtio-net-pci");
+                     PCI_DEVFN(0, 1), "virtio-net-pci");
 
     qdev_set_nic_properties(&eth->qdev, &nd_table[0]);
     qdev_init_nofail(&eth->qdev);