On 03.12.2020 02:58, Igor Druzhinin wrote:
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1641,6 +1641,15 @@ This option is ignored in **pv-shim** mode.
> ### nr_irqs (x86)
> > `= <integer>`
>
> +### irq_max_guests (x86)
As a rule of thumb, new options want to use - instead of _ as
separators. There was a respective discussion quite some time
ago. My patch to treat - and _ equally when parsing options
wasn't liked by Andrew ...
> @@ -435,6 +439,9 @@ int __init init_irq_data(void)
> for ( ; irq < nr_irqs; irq++ )
> irq_to_desc(irq)->irq = irq;
>
> + if ( !irq_max_guests || irq_max_guests > 255)
The 255 is a consequence of the struct field being u8, aiui?
There should be a direct connection between the two, i.e. when
changing the underlying property preferably only one place
should require touching (possible e.g. by converting to a bit
field with its width established via a #define), or comments
on either side should point at the other one.
Also as a nit, there's a blank missing ahead of the closing
paren.
> @@ -1564,7 +1570,8 @@ int pirq_guest_bind(struct vcpu *v, struct pirq *pirq,
> int will_share)
> if ( newaction == NULL )
> {
> spin_unlock_irq(&desc->lock);
> - if ( (newaction = xmalloc(irq_guest_action_t)) != NULL &&
> + if ( (newaction = xmalloc_bytes(sizeof(irq_guest_action_t) +
> + irq_max_guests * sizeof(action->guest[0]))) != NULL &&
As said in the (later) reply to v1, please try to make use of
xmalloc_flex_struct() here.
> @@ -1633,11 +1640,11 @@ int pirq_guest_bind(struct vcpu *v, struct pirq
> *pirq, int will_share)
> goto retry;
> }
>
> - if ( action->nr_guests == IRQ_MAX_GUESTS )
> + if ( action->nr_guests == irq_max_guests )
> {
> printk(XENLOG_G_INFO "Cannot bind IRQ%d to dom%d. "
> - "Already at max share.\n",
> - pirq->pirq, v->domain->domain_id);
> + "Already at max share %u, increase with irq_max_guests=
> option.\n",
> + pirq->pirq, v->domain->domain_id, irq_max_guests);
If you already need to largely redo this printk(), please
- switch to %pd at the same time,
- don't have the format string extend across multiple lines,
- preferably avoid to use full stops (we don't use any in the vast
majority of log messages), e.g. by making it "cannot bind IRQ%d
to %pd, already at max share %u (increase with irq-max-guests=
option)", if you want to stay close to the original wording (I
think this could be expressed more compactly as well).
Jan
