* Re: [PATCH v4 10/10] net/ps3_gelic: Fix DMA mapping problems
From: Christophe Leroy @ 2021-07-25 18:38 UTC (permalink / raw)
To: Geoff Levand; +Cc: netdev, linuxppc-dev, David S. Miller, Jakub Kicinski
In-Reply-To: <7aa1d9b1b4ffadcbdc6f88e4f8d4a323da307595.1627068552.git.geoff@infradead.org>
Geoff Levand <geoff@infradead.org> a écrit :
> Fixes several DMA mapping problems with the PS3's gelic network driver:
>
> * Change from checking the return value of dma_map_single to using the
> dma_mapping_error routine.
> * Use the correct buffer length when mapping the RX skb.
> * Improved error checking and debug logging.
The patch is quite big and probably deserves more explanation. For
instance, explain why the buffer length is not correct today.
Also as it is a bug fixing patch, it should include a 'fixes' tag, and
a Cc: to stable@vger.kernel.org. Also, when possible, bug fixes should
be one of the first patches in a series like that so that they can be
applied to stable without applying the whole series.
Christophe
>
> Fixes runtime errors like these, and also other randomly occurring errors:
>
> IP-Config: Complete:
> DMA-API: ps3_gelic_driver sb_05: device driver failed to check map error
> WARNING: CPU: 0 PID: 0 at kernel/dma/debug.c:1027 .check_unmap+0x888/0x8dc
>
> Signed-off-by: Geoff Levand <geoff@infradead.org>
> ---
> drivers/net/ethernet/toshiba/ps3_gelic_net.c | 183 +++++++++++--------
> 1 file changed, 108 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> index 42f4de9ad5fe..11ddeacb1159 100644
> --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c
> @@ -336,22 +336,31 @@ static int gelic_card_init_chain(struct
> gelic_card *card,
> struct gelic_descr_chain *chain, struct gelic_descr *start_descr,
> int descr_count)
> {
> - int i;
> - struct gelic_descr *descr;
> + struct gelic_descr *descr = start_descr;
> struct device *dev = ctodev(card);
> + unsigned int index;
>
> - descr = start_descr;
> - memset(descr, 0, sizeof(*descr) *descr_count);
> + memset(start_descr, 0, descr_count * sizeof(*start_descr));
>
> - for (i = 0; i < descr_count; i++, descr++) {
> - descr->link.size = sizeof(struct gelic_hw_regs);
> + for (index = 0, descr = start_descr; index < descr_count;
> + index++, descr++) {
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> - descr->link.cpu_addr =
> - dma_map_single(dev, descr, descr->link.size,
> - DMA_BIDIRECTIONAL);
>
> - if (!descr->link.cpu_addr)
> - goto iommu_error;
> + descr->link.size = sizeof(struct gelic_hw_regs);
> + descr->link.cpu_addr = dma_map_single(dev, descr,
> + descr->link.size, DMA_BIDIRECTIONAL);
> +
> + if (unlikely(dma_mapping_error(dev, descr->link.cpu_addr))) {
> + dev_err(dev, "%s:%d: dma_mapping_error\n", __func__,
> + __LINE__);
> +
> + for (index--, descr--; index > 0; index--, descr--) {
> + if (descr->link.cpu_addr) {
> + gelic_unmap_link(dev, descr);
> + }
> + }
> + return -ENOMEM;
> + }
>
> descr->next = descr + 1;
> descr->prev = descr - 1;
> @@ -360,8 +369,9 @@ static int gelic_card_init_chain(struct gelic_card *card,
> (descr - 1)->next = start_descr;
> start_descr->prev = (descr - 1);
>
> - descr = start_descr;
> - for (i = 0; i < descr_count; i++, descr++) {
> + /* chain bus addr of hw descriptor */
> + for (index = 0, descr = start_descr; index < descr_count;
> + index++, descr++) {
> descr->hw_regs.next_descr_addr =
> cpu_to_be32(descr->next->link.cpu_addr);
> }
> @@ -373,12 +383,6 @@ static int gelic_card_init_chain(struct
> gelic_card *card,
> (descr - 1)->hw_regs.next_descr_addr = 0;
>
> return 0;
> -
> -iommu_error:
> - for (i--, descr--; 0 <= i; i--, descr--)
> - if (descr->link.cpu_addr)
> - gelic_unmap_link(dev, descr);
> - return -ENOMEM;
> }
>
> /**
> @@ -395,49 +399,63 @@ static int gelic_descr_prepare_rx(struct
> gelic_card *card,
> struct gelic_descr *descr)
> {
> struct device *dev = ctodev(card);
> - int offset;
> - unsigned int bufsize;
> + struct aligned_buff {
> + unsigned int total_bytes;
> + unsigned int offset;
> + };
> + struct aligned_buff a_buf;
> + dma_addr_t cpu_addr;
>
> if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE) {
> dev_err(dev, "%s:%d: ERROR status\n", __func__, __LINE__);
> }
>
> - /* we need to round up the buffer size to a multiple of 128 */
> - bufsize = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN);
> + a_buf.total_bytes = ALIGN(GELIC_NET_MAX_MTU, GELIC_NET_RXBUF_ALIGN)
> + + GELIC_NET_RXBUF_ALIGN;
> +
> + descr->skb = dev_alloc_skb(a_buf.total_bytes);
>
> - /* and we need to have it 128 byte aligned, therefore we allocate a
> - * bit more */
> - descr->skb = dev_alloc_skb(bufsize + GELIC_NET_RXBUF_ALIGN - 1);
> if (!descr->skb) {
> - descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> return -ENOMEM;
> }
> - descr->hw_regs.payload.size = cpu_to_be32(bufsize);
> +
> + a_buf.offset = PTR_ALIGN(descr->skb->data, GELIC_NET_RXBUF_ALIGN)
> + - descr->skb->data;
> +
> + if (a_buf.offset) {
> + dev_dbg(dev, "%s:%d: offset=%u\n", __func__, __LINE__,
> + a_buf.offset);
> + skb_reserve(descr->skb, a_buf.offset);
> + }
> +
> descr->hw_regs.dmac_cmd_status = 0;
> descr->hw_regs.result_size = 0;
> descr->hw_regs.valid_size = 0;
> descr->hw_regs.data_error = 0;
>
> - offset = ((unsigned long)descr->skb->data) &
> - (GELIC_NET_RXBUF_ALIGN - 1);
> - if (offset)
> - skb_reserve(descr->skb, GELIC_NET_RXBUF_ALIGN - offset);
> - /* io-mmu-map the skb */
> - descr->hw_regs.payload.dev_addr = cpu_to_be32(dma_map_single(dev,
> - descr->skb->data,
> - GELIC_NET_MAX_MTU,
> - DMA_FROM_DEVICE));
> - if (!descr->hw_regs.payload.dev_addr) {
> + descr->hw_regs.payload.size = a_buf.total_bytes - a_buf.offset;
> + cpu_addr = dma_map_single(dev, descr->skb->data,
> + descr->hw_regs.payload.size, DMA_FROM_DEVICE);
> + descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
> +
> + if (unlikely(dma_mapping_error(dev, cpu_addr))) {
> + dev_err(dev, "%s:%d: dma_mapping_error\n", __func__, __LINE__);
> +
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> +
> dev_kfree_skb_any(descr->skb);
> descr->skb = NULL;
> - dev_info(dev,
> - "%s:Could not iommu-map rx buffer\n", __func__);
> +
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> +
> return -ENOMEM;
> - } else {
> - gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
> - return 0;
> }
> +
> + gelic_descr_set_status(descr, GELIC_DESCR_DMA_CARDOWNED);
> + return 0;
> }
>
> /**
> @@ -454,13 +472,18 @@ static void gelic_card_release_rx_chain(struct
> gelic_card *card)
> if (descr->skb) {
> dma_unmap_single(dev,
> be32_to_cpu(descr->hw_regs.payload.dev_addr),
> - descr->skb->len, DMA_FROM_DEVICE);
> - descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size, DMA_FROM_DEVICE);
> +
> dev_kfree_skb_any(descr->skb);
> descr->skb = NULL;
> +
> gelic_descr_set_status(descr,
> GELIC_DESCR_DMA_NOT_IN_USE);
> }
> +
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> +
> descr = descr->next;
> } while (descr != card->rx_chain.head);
> }
> @@ -526,17 +549,19 @@ static void gelic_descr_release_tx(struct
> gelic_card *card,
> GELIC_DESCR_TX_TAIL));
>
> dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
> - skb->len, DMA_TO_DEVICE);
> - dev_kfree_skb_any(skb);
> + descr->hw_regs.payload.size, DMA_TO_DEVICE);
>
> descr->hw_regs.payload.dev_addr = 0;
> descr->hw_regs.payload.size = 0;
> +
> + dev_kfree_skb_any(skb);
> + descr->skb = NULL;
> +
> descr->hw_regs.next_descr_addr = 0;
> descr->hw_regs.result_size = 0;
> descr->hw_regs.valid_size = 0;
> descr->hw_regs.data_status = 0;
> descr->hw_regs.data_error = 0;
> - descr->skb = NULL;
>
> gelic_descr_set_status(descr, GELIC_DESCR_DMA_NOT_IN_USE);
> }
> @@ -565,31 +590,34 @@ static void gelic_card_wake_queues(struct
> gelic_card *card)
> static void gelic_card_release_tx_chain(struct gelic_card *card, int stop)
> {
> struct gelic_descr_chain *tx_chain;
> - enum gelic_descr_dma_status status;
> struct device *dev = ctodev(card);
> - struct net_device *netdev;
> - int release = 0;
> + int release;
> +
> + for (release = 0, tx_chain = &card->tx_chain;
> + tx_chain->head != tx_chain->tail && tx_chain->tail;
> + tx_chain->tail = tx_chain->tail->next) {
> + enum gelic_descr_dma_status status;
> + struct gelic_descr *descr;
> + struct net_device *netdev;
> +
> + descr = tx_chain->tail;
> + status = gelic_descr_get_status(descr);
> + netdev = descr->skb->dev;
>
> - for (tx_chain = &card->tx_chain;
> - tx_chain->head != tx_chain->tail && tx_chain->tail;
> - tx_chain->tail = tx_chain->tail->next) {
> - status = gelic_descr_get_status(tx_chain->tail);
> - netdev = tx_chain->tail->skb->dev;
> switch (status) {
> case GELIC_DESCR_DMA_RESPONSE_ERROR:
> case GELIC_DESCR_DMA_PROTECTION_ERROR:
> case GELIC_DESCR_DMA_FORCE_END:
> - dev_info_ratelimited(dev,
> - "%s:%d: forcing end of tx descriptor with status %x\n",
> - __func__, __LINE__, status);
> + dev_info_ratelimited(dev,
> + "%s:%d: forcing end of tx descriptor with status %x\n",
> + __func__, __LINE__, status);
> netdev->stats.tx_dropped++;
> break;
>
> case GELIC_DESCR_DMA_COMPLETE:
> - if (tx_chain->tail->skb) {
> + if (descr->skb) {
> netdev->stats.tx_packets++;
> - netdev->stats.tx_bytes +=
> - tx_chain->tail->skb->len;
> + netdev->stats.tx_bytes += descr->skb->len;
> }
> break;
>
> @@ -599,7 +627,7 @@ static void gelic_card_release_tx_chain(struct
> gelic_card *card, int stop)
> }
> }
>
> - gelic_descr_release_tx(card, tx_chain->tail);
> + gelic_descr_release_tx(card, descr);
> release++;
> }
> out:
> @@ -703,19 +731,19 @@ int gelic_net_stop(struct net_device *netdev)
> *
> * returns the address of the next descriptor, or NULL if not available.
> */
> -static struct gelic_descr *
> -gelic_card_get_next_tx_descr(struct gelic_card *card)
> +static struct gelic_descr *gelic_card_get_next_tx_descr(struct
> gelic_card *card)
> {
> if (!card->tx_chain.head)
> return NULL;
> +
> /* see if the next descriptor is free */
> if (card->tx_chain.tail != card->tx_chain.head->next &&
> - gelic_descr_get_status(card->tx_chain.head) ==
> - GELIC_DESCR_DMA_NOT_IN_USE)
> + (gelic_descr_get_status(card->tx_chain.head) ==
> + GELIC_DESCR_DMA_NOT_IN_USE)) {
> return card->tx_chain.head;
> - else
> - return NULL;
> + }
>
> + return NULL;
> }
>
> /**
> @@ -809,18 +837,23 @@ static int gelic_descr_prepare_tx(struct
> gelic_card *card,
> if (!skb_tmp) {
> return -ENOMEM;
> }
> +
> skb = skb_tmp;
> }
>
> - cpu_addr = dma_map_single(dev, skb->data, skb->len, DMA_TO_DEVICE);
> + descr->hw_regs.payload.size = skb->len;
> + cpu_addr = dma_map_single(dev, skb->data, descr->hw_regs.payload.size,
> + DMA_TO_DEVICE);
> + descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
>
> - if (!cpu_addr) {
> + if (unlikely(dma_mapping_error(dev, cpu_addr))) {
> dev_err(dev, "%s:%d: dma_mapping_error\n", __func__, __LINE__);
> +
> + descr->hw_regs.payload.dev_addr = 0;
> + descr->hw_regs.payload.size = 0;
> return -ENOMEM;
> }
>
> - descr->hw_regs.payload.dev_addr = cpu_to_be32(cpu_addr);
> - descr->hw_regs.payload.size = cpu_to_be32(skb->len);
> descr->skb = skb;
> descr->hw_regs.data_status = 0;
> descr->hw_regs.next_descr_addr = 0; /* terminate hw descr */
> @@ -948,9 +981,9 @@ static void gelic_net_pass_skb_up(struct
> gelic_descr *descr,
>
> data_status = be32_to_cpu(descr->hw_regs.data_status);
> data_error = be32_to_cpu(descr->hw_regs.data_error);
> - /* unmap skb buffer */
> +
> dma_unmap_single(dev, be32_to_cpu(descr->hw_regs.payload.dev_addr),
> - GELIC_NET_MAX_MTU, DMA_FROM_DEVICE);
> + descr->hw_regs.payload.size, DMA_FROM_DEVICE);
>
> skb_put(skb, be32_to_cpu(descr->hw_regs.valid_size) ?
> be32_to_cpu(descr->hw_regs.valid_size) :
> --
> 2.25.1
^ permalink raw reply
* Re: [PATCH v4 1/2] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Nicholas Piggin @ 2021-07-26 1:50 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <20210722221240.2384655-2-farosas@linux.ibm.com>
Excerpts from Fabiano Rosas's message of July 23, 2021 8:12 am:
> As one of the arguments of the H_ENTER_NESTED hypercall, the nested
> hypervisor (L1) prepares a structure containing the values of various
> hypervisor-privileged registers with which it wants the nested guest
> (L2) to run. Since the nested HV runs in supervisor mode it needs the
> host to write to these registers.
>
> To stop a nested HV manipulating this mechanism and using a nested
> guest as a proxy to access a facility that has been made unavailable
> to it, we have a routine that sanitises the values of the HV registers
> before copying them into the nested guest's vcpu struct.
>
> However, when coming out of the guest the values are copied as they
> were back into L1 memory, which means that any sanitisation we did
> during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
>
> This patch alters this sanitisation to have effect on the vcpu->arch
> registers directly before entering and after exiting the guest,
> leaving the structure that is copied back into L1 unchanged (except
> when we really want L1 to access the value, e.g the Cause bits of
> HFSCR).
These patches look good to me. I ported my demand-faulting patches on
top of them and things seem to work okay.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Just one minor nit:
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv_nested.c | 100 +++++++++++++++-------------
> 1 file changed, 52 insertions(+), 48 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> index 8543ad538b0c..3804dc50ebe8 100644
> --- a/arch/powerpc/kvm/book3s_hv_nested.c
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -104,8 +104,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> {
> struct kvmppc_vcore *vc = vcpu->arch.vcore;
>
> + /*
> + * When loading the hypervisor-privileged registers to run L2,
> + * we might have used bits from L1 state to restrict what the
> + * L2 state is allowed to be. Since L1 is not allowed to read
> + * the HV registers, do not include these modifications in the
> + * return state.
> + */
> + hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
> + (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
Can you change this to only update HFSCR intr cause field when we take a
hfac interrupt? It's possible the L0 can cause other kinds of hfacs
behind the back of the L1 with demand faulting, so it would be unusual
for L1 to see the register change if it didn't take an hfac interrupt.
Thanks,
Nick
> +
> hr->dpdes = vc->dpdes;
> - hr->hfscr = vcpu->arch.hfscr;
> hr->purr = vcpu->arch.purr;
> hr->spurr = vcpu->arch.spurr;
> hr->ic = vcpu->arch.ic;
> @@ -134,49 +143,7 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
> }
> }
>
> -/*
> - * This can result in some L0 HV register state being leaked to an L1
> - * hypervisor when the hv_guest_state is copied back to the guest after
> - * being modified here.
> - *
> - * There is no known problem with such a leak, and in many cases these
> - * register settings could be derived by the guest by observing behaviour
> - * and timing, interrupts, etc., but it is an issue to consider.
> - */
> -static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
> -{
> - struct kvmppc_vcore *vc = vcpu->arch.vcore;
> - u64 mask;
> -
> - /*
> - * Don't let L1 change LPCR bits for the L2 except these:
> - */
> - mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
> - LPCR_LPES | LPCR_MER;
> -
> - /*
> - * Additional filtering is required depending on hardware
> - * and configuration.
> - */
> - hr->lpcr = kvmppc_filter_lpcr_hv(vcpu->kvm,
> - (vc->lpcr & ~mask) | (hr->lpcr & mask));
> -
> - /*
> - * Don't let L1 enable features for L2 which we've disabled for L1,
> - * but preserve the interrupt cause field.
> - */
> - hr->hfscr &= (HFSCR_INTR_CAUSE | vcpu->arch.hfscr);
> -
> - /* Don't let data address watchpoint match in hypervisor state */
> - hr->dawrx0 &= ~DAWRX_HYP;
> - hr->dawrx1 &= ~DAWRX_HYP;
> -
> - /* Don't let completed instruction address breakpt match in HV state */
> - if ((hr->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
> - hr->ciabr &= ~CIABR_PRIV;
> -}
> -
> -static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
> +static void restore_hv_regs(struct kvm_vcpu *vcpu, const struct hv_guest_state *hr)
> {
> struct kvmppc_vcore *vc = vcpu->arch.vcore;
>
> @@ -288,6 +255,43 @@ static int kvmhv_write_guest_state_and_regs(struct kvm_vcpu *vcpu,
> sizeof(struct pt_regs));
> }
>
> +static void load_l2_hv_regs(struct kvm_vcpu *vcpu,
> + const struct hv_guest_state *l2_hv,
> + const struct hv_guest_state *l1_hv, u64 *lpcr)
> +{
> + struct kvmppc_vcore *vc = vcpu->arch.vcore;
> + u64 mask;
> +
> + restore_hv_regs(vcpu, l2_hv);
> +
> + /*
> + * Don't let L1 change LPCR bits for the L2 except these:
> + */
> + mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
> + LPCR_LPES | LPCR_MER;
> +
> + /*
> + * Additional filtering is required depending on hardware
> + * and configuration.
> + */
> + *lpcr = kvmppc_filter_lpcr_hv(vcpu->kvm,
> + (vc->lpcr & ~mask) | (*lpcr & mask));
> +
> + /*
> + * Don't let L1 enable features for L2 which we've disabled for L1,
> + * but preserve the interrupt cause field.
> + */
> + vcpu->arch.hfscr = l2_hv->hfscr & (HFSCR_INTR_CAUSE | l1_hv->hfscr);
> +
> + /* Don't let data address watchpoint match in hypervisor state */
> + vcpu->arch.dawrx0 = l2_hv->dawrx0 & ~DAWRX_HYP;
> + vcpu->arch.dawrx1 = l2_hv->dawrx1 & ~DAWRX_HYP;
> +
> + /* Don't let completed instruction address breakpt match in HV state */
> + if ((l2_hv->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
> + vcpu->arch.ciabr = l2_hv->ciabr & ~CIABR_PRIV;
> +}
> +
> long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> {
> long int err, r;
> @@ -296,7 +300,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> struct hv_guest_state l2_hv = {0}, saved_l1_hv;
> struct kvmppc_vcore *vc = vcpu->arch.vcore;
> u64 hv_ptr, regs_ptr;
> - u64 hdec_exp;
> + u64 hdec_exp, lpcr;
> s64 delta_purr, delta_spurr, delta_ic, delta_vtb;
>
> if (vcpu->kvm->arch.l1_ptcr == 0)
> @@ -349,8 +353,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> /* Guest must always run with ME enabled, HV disabled. */
> vcpu->arch.shregs.msr = (vcpu->arch.regs.msr | MSR_ME) & ~MSR_HV;
>
> - sanitise_hv_regs(vcpu, &l2_hv);
> - restore_hv_regs(vcpu, &l2_hv);
> + lpcr = l2_hv.lpcr;
> + load_l2_hv_regs(vcpu, &l2_hv, &saved_l1_hv, &lpcr);
>
> vcpu->arch.ret = RESUME_GUEST;
> vcpu->arch.trap = 0;
> @@ -360,7 +364,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
> r = RESUME_HOST;
> break;
> }
> - r = kvmhv_run_single_vcpu(vcpu, hdec_exp, l2_hv.lpcr);
> + r = kvmhv_run_single_vcpu(vcpu, hdec_exp, lpcr);
> } while (is_kvmppc_resume_guest(r));
>
> /* save L2 state for return */
> --
> 2.29.2
>
>
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS d9c57d3ed52a92536f5fa59dc5ccdd58b4875076
From: kernel test robot @ 2021-07-26 2:33 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: d9c57d3ed52a92536f5fa59dc5ccdd58b4875076 KVM: PPC: Book3S HV Nested: Sanitise H_ENTER_NESTED TM state
elapsed time: 724m
configs tested: 158
configs skipped: 99
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 defconfig
arm allmodconfig
arm allyesconfig
arm64 allyesconfig
i386 randconfig-c001-20210725
i386 randconfig-c001-20210726
arm pxa_defconfig
arm spear6xx_defconfig
mips mtx1_defconfig
powerpc cm5200_defconfig
arm dove_defconfig
powerpc mpc85xx_cds_defconfig
x86_64 allnoconfig
h8300 h8s-sim_defconfig
powerpc mvme5100_defconfig
mips rs90_defconfig
alpha defconfig
ia64 bigsur_defconfig
openrisc simple_smp_defconfig
sh sh03_defconfig
powerpc rainier_defconfig
arm ixp4xx_defconfig
powerpc mpc8315_rdb_defconfig
powerpc microwatt_defconfig
powerpc akebono_defconfig
sh se7206_defconfig
m68k atari_defconfig
m68k sun3x_defconfig
mips jmr3927_defconfig
mips ath79_defconfig
powerpc mgcoge_defconfig
um x86_64_defconfig
powerpc g5_defconfig
microblaze defconfig
mips nlm_xlp_defconfig
powerpc allnoconfig
mips bmips_be_defconfig
powerpc ppc40x_defconfig
mips decstation_64_defconfig
sh migor_defconfig
powerpc64 alldefconfig
powerpc obs600_defconfig
sh sh7710voipgw_defconfig
sh rsk7203_defconfig
nios2 allyesconfig
powerpc ep8248e_defconfig
parisc generic-32bit_defconfig
riscv nommu_k210_sdcard_defconfig
ia64 zx1_defconfig
powerpc pseries_defconfig
riscv nommu_virt_defconfig
powerpc storcenter_defconfig
mips maltaup_xpa_defconfig
mips cobalt_defconfig
sh se7780_defconfig
arm pcm027_defconfig
xtensa defconfig
h8300 alldefconfig
powerpc ksi8560_defconfig
m68k stmark2_defconfig
powerpc tqm8541_defconfig
powerpc ep88xc_defconfig
sh apsh4a3a_defconfig
powerpc sequoia_defconfig
xtensa iss_defconfig
mips pistachio_defconfig
sh landisk_defconfig
arc nsim_700_defconfig
powerpc mpc834x_mds_defconfig
sparc64 alldefconfig
arm magician_defconfig
powerpc iss476-smp_defconfig
nios2 defconfig
sh ap325rxa_defconfig
mips malta_kvm_defconfig
m68k amcore_defconfig
ia64 defconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
csky defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
sparc defconfig
i386 defconfig
i386 allyesconfig
sparc allyesconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
i386 randconfig-a005-20210725
i386 randconfig-a003-20210725
i386 randconfig-a004-20210725
i386 randconfig-a002-20210725
i386 randconfig-a001-20210725
i386 randconfig-a006-20210725
i386 randconfig-a005-20210726
i386 randconfig-a003-20210726
i386 randconfig-a004-20210726
i386 randconfig-a002-20210726
i386 randconfig-a001-20210726
i386 randconfig-a006-20210726
x86_64 randconfig-a011-20210725
x86_64 randconfig-a016-20210725
x86_64 randconfig-a013-20210725
x86_64 randconfig-a014-20210725
x86_64 randconfig-a012-20210725
x86_64 randconfig-a015-20210725
i386 randconfig-a016-20210725
i386 randconfig-a013-20210725
i386 randconfig-a012-20210725
i386 randconfig-a014-20210725
i386 randconfig-a011-20210725
i386 randconfig-a015-20210725
i386 randconfig-a016-20210726
i386 randconfig-a013-20210726
i386 randconfig-a012-20210726
i386 randconfig-a011-20210726
i386 randconfig-a014-20210726
i386 randconfig-a015-20210726
riscv nommu_k210_defconfig
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um i386_defconfig
x86_64 allyesconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-c001-20210726
x86_64 randconfig-c001-20210725
x86_64 randconfig-a003-20210725
x86_64 randconfig-a006-20210725
x86_64 randconfig-a001-20210725
x86_64 randconfig-a005-20210725
x86_64 randconfig-a004-20210725
x86_64 randconfig-a002-20210725
x86_64 randconfig-a011-20210726
x86_64 randconfig-a013-20210726
x86_64 randconfig-a014-20210726
x86_64 randconfig-a012-20210726
x86_64 randconfig-a015-20210726
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS e4277861b65960a264040663ac44c0b946ab402b
From: kernel test robot @ 2021-07-26 2:33 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge
branch HEAD: e4277861b65960a264040663ac44c0b946ab402b Automatic merge of 'fixes' into merge (2021-07-25 23:55)
elapsed time: 722m
configs tested: 166
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
i386 randconfig-c001-20210725
i386 randconfig-c001-20210726
m68k hp300_defconfig
sh kfr2r09_defconfig
m68k sun3x_defconfig
arm mvebu_v7_defconfig
powerpc tqm8540_defconfig
arm pxa_defconfig
arm spear6xx_defconfig
mips mtx1_defconfig
powerpc cm5200_defconfig
arm dove_defconfig
sh sh03_defconfig
powerpc rainier_defconfig
arm ixp4xx_defconfig
powerpc mpc8315_rdb_defconfig
sh landisk_defconfig
sh edosk7760_defconfig
powerpc microwatt_defconfig
mips ath25_defconfig
xtensa alldefconfig
ia64 tiger_defconfig
m68k bvme6000_defconfig
arm spitz_defconfig
mips maltaup_xpa_defconfig
ia64 gensparse_defconfig
powerpc icon_defconfig
powerpc akebono_defconfig
sh se7206_defconfig
m68k atari_defconfig
mips jmr3927_defconfig
mips ath79_defconfig
mips bmips_be_defconfig
powerpc ppc40x_defconfig
mips decstation_64_defconfig
sh migor_defconfig
powerpc64 alldefconfig
arm tct_hammer_defconfig
sh se7780_defconfig
sh rsk7203_defconfig
nios2 allyesconfig
powerpc ep8248e_defconfig
parisc generic-32bit_defconfig
riscv nommu_k210_sdcard_defconfig
ia64 zx1_defconfig
powerpc pseries_defconfig
riscv nommu_virt_defconfig
powerpc storcenter_defconfig
mips cobalt_defconfig
powerpc mgcoge_defconfig
arm pcm027_defconfig
xtensa defconfig
h8300 alldefconfig
powerpc ksi8560_defconfig
m68k stmark2_defconfig
powerpc tqm8541_defconfig
powerpc ep88xc_defconfig
sh apsh4a3a_defconfig
powerpc sequoia_defconfig
mips pistachio_defconfig
arc nsim_700_defconfig
powerpc mpc834x_mds_defconfig
sparc64 alldefconfig
arm magician_defconfig
xtensa iss_defconfig
powerpc iss476-smp_defconfig
nios2 defconfig
sh ap325rxa_defconfig
mips malta_kvm_defconfig
m68k amcore_defconfig
um x86_64_defconfig
arm shannon_defconfig
xtensa xip_kc705_defconfig
openrisc defconfig
arc hsdk_defconfig
x86_64 allnoconfig
ia64 defconfig
ia64 allyesconfig
ia64 allmodconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
s390 defconfig
sparc defconfig
i386 defconfig
i386 allyesconfig
sparc allyesconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a005-20210725
i386 randconfig-a003-20210725
i386 randconfig-a004-20210725
i386 randconfig-a002-20210725
i386 randconfig-a001-20210725
i386 randconfig-a006-20210725
i386 randconfig-a005-20210726
i386 randconfig-a003-20210726
i386 randconfig-a004-20210726
i386 randconfig-a002-20210726
i386 randconfig-a001-20210726
i386 randconfig-a006-20210726
x86_64 randconfig-a011-20210725
x86_64 randconfig-a016-20210725
x86_64 randconfig-a013-20210725
x86_64 randconfig-a014-20210725
x86_64 randconfig-a012-20210725
x86_64 randconfig-a015-20210725
i386 randconfig-a016-20210725
i386 randconfig-a013-20210725
i386 randconfig-a012-20210725
i386 randconfig-a014-20210725
i386 randconfig-a011-20210725
i386 randconfig-a015-20210725
i386 randconfig-a016-20210726
i386 randconfig-a013-20210726
i386 randconfig-a012-20210726
i386 randconfig-a011-20210726
i386 randconfig-a014-20210726
i386 randconfig-a015-20210726
riscv nommu_k210_defconfig
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um i386_defconfig
x86_64 allyesconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-c001-20210726
x86_64 randconfig-c001-20210725
x86_64 randconfig-a003-20210725
x86_64 randconfig-a006-20210725
x86_64 randconfig-a001-20210725
x86_64 randconfig-a005-20210725
x86_64 randconfig-a004-20210725
x86_64 randconfig-a002-20210725
x86_64 randconfig-a011-20210726
x86_64 randconfig-a013-20210726
x86_64 randconfig-a014-20210726
x86_64 randconfig-a012-20210726
x86_64 randconfig-a015-20210726
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v5 1/6] powerpc/pseries: rename min_common_depth to primary_domain_index
From: David Gibson @ 2021-07-26 2:28 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Nathan Lynch, Daniel Henrique Barboza, linuxppc-dev
In-Reply-To: <30f70122-e8e0-e823-c585-bc79ba2e8152@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 1905 bytes --]
On Thu, Jul 22, 2021 at 10:47:49AM +0530, Aneesh Kumar K.V wrote:
> On 7/22/21 8:06 AM, David Gibson wrote:
> > On Thu, Jul 22, 2021 at 11:59:15AM +1000, David Gibson wrote:
> > > On Mon, Jun 28, 2021 at 08:41:12PM +0530, Aneesh Kumar K.V wrote:
> > > > No functional change in this patch.
> > >
> > > The new name does not match how you describe "primary domain index" in
> > > the documentation from patch 6/6. There it comes from the values in
> > > associativity-reference-points, but here it simply comes from the
> > > lengths of all the associativity properties.
> >
> > No, sorry, I misread this code... misled by the old name, so it's a
> > good thing you're changing it.
> >
> > But.. I'm still not sure the new name is accurate, either...
> >
> > [snip]
> > > > if (form1_affinity) {
> > > > - depth = of_read_number(distance_ref_points, 1);
> > > > + index = of_read_number(distance_ref_points, 1);
> >
> > AFACIT distance_ref_points hasn't been altered from the
> > of_get_property() at this point, so isn't this setting depth / index
> > to the number of entries in ref-points, rather than the value of the
> > first entry (which is what primary domain index is supposed to be).
> >
>
> ibm,associativity-reference-points property format is as below.
>
> # lsprop ibm,associativity-reference-points
> ibm,associativity-reference-points
> 00000004 00000002
>
> it doesn't have the number of elements as the first item.
>
> For FORM1 1 element is the NUMA boundary index/primary_domain_index
> For FORM0 2 element is the NUMA boundary index/primary_domain_index.
Sorry, my bad. I foolishly expected consistency from PAPR.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 5/6] powerpc/pseries: Add a helper for form1 cpu distance
From: David Gibson @ 2021-07-26 2:38 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Nathan Lynch, Daniel Henrique Barboza, linuxppc-dev
In-Reply-To: <87wnpiu8ns.fsf@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 4702 bytes --]
On Thu, Jul 22, 2021 at 12:39:27PM +0530, Aneesh Kumar K.V wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
>
> > On Mon, Jun 28, 2021 at 08:41:16PM +0530, Aneesh Kumar K.V wrote:
> >> This helper is only used with the dispatch trace log collection.
> >> A later patch will add Form2 affinity support and this change helps
> >> in keeping that simpler. Also add a comment explaining we don't expect
> >> the code to be called with FORM0
> >>
> >> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> >
> > What makes it a "relative_distance" rather than just a "distance"?
>
> I added that to indicate that the function is not returning the actual
> distance but a number indicative of 'near', 'far' etc. (it actually returns
> 1, 2 etc).
Hm... ok. To me at least it doesn't really convey that meaning, but
then I'm not sure what would. To be "relative distance" means the
distance relative to some other object, but then all the NUMA
distances are that - the distance of one node relative to another.
> >> ---
> >> arch/powerpc/include/asm/topology.h | 4 ++--
> >> arch/powerpc/mm/numa.c | 10 +++++++++-
> >> arch/powerpc/platforms/pseries/lpar.c | 4 ++--
> >> 3 files changed, 13 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> >> index e4db64c0e184..ac8b5ed79832 100644
> >> --- a/arch/powerpc/include/asm/topology.h
> >> +++ b/arch/powerpc/include/asm/topology.h
> >> @@ -36,7 +36,7 @@ static inline int pcibus_to_node(struct pci_bus *bus)
> >> cpu_all_mask : \
> >> cpumask_of_node(pcibus_to_node(bus)))
> >>
> >> -extern int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc);
> >> +int cpu_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc);
> >> extern int __node_distance(int, int);
> >> #define node_distance(a, b) __node_distance(a, b)
> >>
> >> @@ -83,7 +83,7 @@ static inline void sysfs_remove_device_from_node(struct device *dev,
> >>
> >> static inline void update_numa_cpu_lookup_table(unsigned int cpu, int node) {}
> >>
> >> -static inline int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> +static inline int cpu_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> {
> >> return 0;
> >> }
> >> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> >> index 7b142f79d600..c6293037a103 100644
> >> --- a/arch/powerpc/mm/numa.c
> >> +++ b/arch/powerpc/mm/numa.c
> >> @@ -166,7 +166,7 @@ static void unmap_cpu_from_node(unsigned long cpu)
> >> }
> >> #endif /* CONFIG_HOTPLUG_CPU || CONFIG_PPC_SPLPAR */
> >>
> >> -int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> +static int __cpu_form1_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> {
> >> int dist = 0;
> >>
> >> @@ -182,6 +182,14 @@ int cpu_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> return dist;
> >> }
> >>
> >> +int cpu_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> +{
> >> + /* We should not get called with FORM0 */
> >> + VM_WARN_ON(affinity_form == FORM0_AFFINITY);
> >> +
> >> + return __cpu_form1_relative_distance(cpu1_assoc, cpu2_assoc);
> >> +}
> >> +
> >> /* must hold reference to node during call */
> >> static const __be32 *of_get_associativity(struct device_node *dev)
> >> {
> >> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
> >> index dab356e3ff87..afefbdfe768d 100644
> >> --- a/arch/powerpc/platforms/pseries/lpar.c
> >> +++ b/arch/powerpc/platforms/pseries/lpar.c
> >> @@ -261,7 +261,7 @@ static int cpu_relative_dispatch_distance(int last_disp_cpu, int cur_disp_cpu)
> >> if (!last_disp_cpu_assoc || !cur_disp_cpu_assoc)
> >> return -EIO;
> >>
> >> - return cpu_distance(last_disp_cpu_assoc, cur_disp_cpu_assoc);
> >> + return cpu_relative_distance(last_disp_cpu_assoc, cur_disp_cpu_assoc);
> >> }
> >>
> >> static int cpu_home_node_dispatch_distance(int disp_cpu)
> >> @@ -281,7 +281,7 @@ static int cpu_home_node_dispatch_distance(int disp_cpu)
> >> if (!disp_cpu_assoc || !vcpu_assoc)
> >> return -EIO;
> >>
> >> - return cpu_distance(disp_cpu_assoc, vcpu_assoc);
> >> + return cpu_relative_distance(disp_cpu_assoc, vcpu_assoc);
> >> }
> >>
> >> static void update_vcpu_disp_stat(int disp_cpu)
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 4/6] powerpc/pseries: Consolidate different NUMA distance update code paths
From: David Gibson @ 2021-07-26 2:37 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Nathan Lynch, Daniel Henrique Barboza, linuxppc-dev
In-Reply-To: <87zgueu8ql.fsf@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 16044 bytes --]
On Thu, Jul 22, 2021 at 12:37:46PM +0530, Aneesh Kumar K.V wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
>
> > On Mon, Jun 28, 2021 at 08:41:15PM +0530, Aneesh Kumar K.V wrote:
> >> The associativity details of the newly added resourced are collected from
> >> the hypervisor via "ibm,configure-connector" rtas call. Update the numa
> >> distance details of the newly added numa node after the above call.
> >>
> >> Instead of updating NUMA distance every time we lookup a node id
> >> from the associativity property, add helpers that can be used
> >> during boot which does this only once. Also remove the distance
> >> update from node id lookup helpers.
> >>
> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> >> ---
> >> arch/powerpc/mm/numa.c | 173 +++++++++++++-----
> >> arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +
> >> .../platforms/pseries/hotplug-memory.c | 2 +
> >> arch/powerpc/platforms/pseries/pseries.h | 1 +
> >> 4 files changed, 132 insertions(+), 46 deletions(-)
> >>
> >> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> >> index 0ec16999beef..7b142f79d600 100644
> >> --- a/arch/powerpc/mm/numa.c
> >> +++ b/arch/powerpc/mm/numa.c
> >> @@ -208,22 +208,6 @@ int __node_distance(int a, int b)
> >> }
> >> EXPORT_SYMBOL(__node_distance);
> >>
> >> -static void initialize_distance_lookup_table(int nid,
> >> - const __be32 *associativity)
> >> -{
> >> - int i;
> >> -
> >> - if (affinity_form != FORM1_AFFINITY)
> >> - return;
> >> -
> >> - for (i = 0; i < max_associativity_domain_index; i++) {
> >> - const __be32 *entry;
> >> -
> >> - entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
> >> - distance_lookup_table[nid][i] = of_read_number(entry, 1);
> >> - }
> >> -}
> >> -
> >> /*
> >> * Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
> >> * info is found.
> >> @@ -241,15 +225,6 @@ static int associativity_to_nid(const __be32 *associativity)
> >> /* POWER4 LPAR uses 0xffff as invalid node */
> >> if (nid == 0xffff || nid >= nr_node_ids)
> >> nid = NUMA_NO_NODE;
> >> -
> >> - if (nid > 0 &&
> >> - of_read_number(associativity, 1) >= max_associativity_domain_index) {
> >> - /*
> >> - * Skip the length field and send start of associativity array
> >> - */
> >> - initialize_distance_lookup_table(nid, associativity + 1);
> >> - }
> >> -
> >> out:
> >> return nid;
> >> }
> >> @@ -287,6 +262,49 @@ int of_node_to_nid(struct device_node *device)
> >> }
> >> EXPORT_SYMBOL(of_node_to_nid);
> >>
> >> +static void __initialize_form1_numa_distance(const __be32 *associativity)
> >> +{
> >> + int i, nid;
> >> +
> >> + if (affinity_form != FORM1_AFFINITY)
> >
> > Since this shouldn't be called on a !form1 system, this could be a WARN_ON().
>
> The way we call functions currently, instead of doing
>
> if (affinity_form == FORM1_AFFINITY)
> __initialize_form1_numa_distance()
>
> We avoid doing the if check in multiple places. For example
> parse_numa_properties will fetch the associativity array to find the
> details of online node and set it online. We use the same code path to
> initialize distance.
>
> if (__vphn_get_associativity(i, vphn_assoc) == 0) {
> nid = associativity_to_nid(vphn_assoc);
> __initialize_form1_numa_distance(vphn_assoc);
> } else {
>
> cpu = of_get_cpu_node(i, NULL);
> BUG_ON(!cpu);
>
> associativity = of_get_associativity(cpu);
> if (associativity) {
> nid = associativity_to_nid(associativity);
> __initialize_form1_numa_distance(associativity);
> }
>
> We avoid the the if (affinity_form == FORM1_AFFINITY) check there by
> moving the check inside __initialize_form1_numa_distance().
Oh.. ok. The only caller I spotted was already doing a test against
affinity_form.
> >> + return;
> >> +
> >> + if (of_read_number(associativity, 1) >= primary_domain_index) {
> >> + nid = of_read_number(&associativity[primary_domain_index], 1);
> >
> > This computes the nid from the assoc array independently of
> > associativity_to_nid, which doesn't seem like a good idea. Wouldn't
> > it be better to call assocaitivity_to_nid(), then make the next bit
> > conditional on nid !== NUMA_NO_NODE?
>
> @@ -302,9 +302,8 @@ static void __initialize_form1_numa_distance(const __be32 *associativity)
> if (affinity_form != FORM1_AFFINITY)
> return;
>
> - if (of_read_number(associativity, 1) >= primary_domain_index) {
> - nid = of_read_number(&associativity[primary_domain_index], 1);
> -
> + nid = associativity_to_nid(associativity);
> + if (nid != NUMA_NO_NODE) {
> for (i = 0; i < distance_ref_points_depth; i++) {
> const __be32 *entry;
Right.
> >
> >> +
> >> + for (i = 0; i < max_associativity_domain_index; i++) {
> >> + const __be32 *entry;
> >> +
> >> + entry = &associativity[be32_to_cpu(distance_ref_points[i])];
> >> + distance_lookup_table[nid][i] = of_read_number(entry, 1);
> >> + }
> >> + }
> >> +}
> >> +
> >> +static void initialize_form1_numa_distance(struct device_node *node)
> >> +{
> >> + const __be32 *associativity;
> >> +
> >> + associativity = of_get_associativity(node);
> >> + if (!associativity)
> >> + return;
> >> +
> >> + __initialize_form1_numa_distance(associativity);
> >> +}
> >> +
> >> +/*
> >> + * Used to update distance information w.r.t newly added node.
> >> + */
> >> +void update_numa_distance(struct device_node *node)
> >> +{
> >> + if (affinity_form == FORM0_AFFINITY)
> >> + return;
> >> + else if (affinity_form == FORM1_AFFINITY) {
> >> + initialize_form1_numa_distance(node);
> >> + return;
> >> + }
> >> +}
> >> +
> >> static int __init find_primary_domain_index(void)
> >> {
> >> int index;
> >> @@ -433,6 +451,48 @@ static int of_get_assoc_arrays(struct assoc_arrays *aa)
> >> return 0;
> >> }
> >>
> >> +static int get_nid_and_numa_distance(struct drmem_lmb *lmb)
> >> +{
> >> + struct assoc_arrays aa = { .arrays = NULL };
> >> + int default_nid = NUMA_NO_NODE;
> >> + int nid = default_nid;
> >> + int rc, index;
> >> +
> >> + if ((primary_domain_index < 0) || !numa_enabled)
> >
> > Under what circumstances could you get primary_domain_index < 0?
>
> IIUC that is to handle failure to parse device tree.
> ea9f5b702fe0215188fba2eda117419e4ae90a67
Ok.
> >
> >> + return default_nid;
Returning NUMA_NO_NODE explicitly, rather than an alias to it might be
clearer here, but it's not a big detail.
> >> +
> >> + rc = of_get_assoc_arrays(&aa);
> >> + if (rc)
> >> + return default_nid;
> >> +
> >> + if (primary_domain_index <= aa.array_sz &&
> >> + !(lmb->flags & DRCONF_MEM_AI_INVALID) && lmb->aa_index < aa.n_arrays) {
> >> + index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
> >
> > Does anywhere verify that primary_domain_index <= aa.array_sz?
>
> That is the first part of the check?
Oh, sorry, missed that. I think I was expecting it to be an early
exit, rather than folded into the rest of this complex condition.
>
> >
> >> + nid = of_read_number(&aa.arrays[index], 1);
> >> +
> >> + if (nid == 0xffff || nid >= nr_node_ids)
> >> + nid = default_nid;
> >> + if (nid > 0 && affinity_form == FORM1_AFFINITY) {
> >> + int i;
> >> + const __be32 *associativity;
> >> +
> >> + index = lmb->aa_index * aa.array_sz;
> >> + associativity = &aa.arrays[index];
> >> + /*
> >> + * lookup array associativity entries have different format
> >> + * There is no length of the array as the first element.
> >
> > The difference it very small, and this is not a hot path. Couldn't
> > you reduce a chunk of code by prepending aa.array_sz, then re-using
> > __initialize_form1_numa_distance. Or even making
> > __initialize_form1_numa_distance() take the length as a parameter.
>
> The changes are small but confusing w.r.t how we look at the
> associativity-lookup-arrays. The way we interpret associativity array
> and associativity lookup array using primary_domain_index is different.
> Hence the '-1' in the node lookup here.
They're really not, though. It's exactly the same interpretation of
the associativity array itself - it's just that one of them has the
array prepended with a (redundant) length. So you can make
__initialize_form1_numa_distance() work on the "bare" associativity
array, with a given length. Here you call it with aa.array_sz as the
length, and in the other place you call it with prop[0] as the length.
>
> index = lmb->aa_index * aa.array_sz + primary_domain_index - 1;
> nid = of_read_number(&aa.arrays[index], 1);
>
>
> >
> >> + */
> >> + for (i = 0; i < max_associativity_domain_index; i++) {
> >> + const __be32 *entry;
> >> +
> >> + entry = &associativity[be32_to_cpu(distance_ref_points[i]) - 1];
> >
> > Does anywhere verify that distance_ref_points[i] <= aa.array_size for
> > every i?
>
> We do check for
>
> if (primary_domain_index <= aa.array_sz &&
Right, but that doesn't check the other distance_ref_points entries.
Not that there's any reason to have extra entries with Form2, but we
still don't want stray array accesses.
>
> >
> >> + distance_lookup_table[nid][i] = of_read_number(entry, 1);
> >> + }
> >> + }
> >> + }
> >> + return nid;
> >> +}
> >> +
> >> /*
> >> * This is like of_node_to_nid_single() for memory represented in the
> >> * ibm,dynamic-reconfiguration-memory node.
> >> @@ -458,21 +518,14 @@ int of_drconf_to_nid_single(struct drmem_lmb *lmb)
> >>
> >> if (nid == 0xffff || nid >= nr_node_ids)
> >> nid = default_nid;
> >> -
> >> - if (nid > 0) {
> >> - index = lmb->aa_index * aa.array_sz;
> >> - initialize_distance_lookup_table(nid,
> >> - &aa.arrays[index]);
> >> - }
> >> }
> >> -
> >> return nid;
> >> }
> >>
> >> #ifdef CONFIG_PPC_SPLPAR
> >> -static int vphn_get_nid(long lcpu)
> >> +
> >> +static int __vphn_get_associativity(long lcpu, __be32 *associativity)
> >> {
> >> - __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> >> long rc, hwid;
> >>
> >> /*
> >> @@ -492,10 +545,22 @@ static int vphn_get_nid(long lcpu)
> >>
> >> rc = hcall_vphn(hwid, VPHN_FLAG_VCPU, associativity);
> >> if (rc == H_SUCCESS)
> >> - return associativity_to_nid(associativity);
> >> + return 0;
> >> }
> >>
> >> + return -1;
> >> +}
> >> +
> >> +static int vphn_get_nid(long lcpu)
> >> +{
> >> + __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> >> +
> >> +
> >> + if (!__vphn_get_associativity(lcpu, associativity))
> >> + return associativity_to_nid(associativity);
> >> +
> >> return NUMA_NO_NODE;
> >> +
> >> }
> >> #else
> >> static int vphn_get_nid(long unused)
> >> @@ -692,7 +757,7 @@ static int __init numa_setup_drmem_lmb(struct drmem_lmb *lmb,
> >> size = read_n_cells(n_mem_size_cells, usm);
> >> }
> >>
> >> - nid = of_drconf_to_nid_single(lmb);
> >> + nid = get_nid_and_numa_distance(lmb);
> >> fake_numa_create_new_node(((base + size) >> PAGE_SHIFT),
> >> &nid);
> >> node_set_online(nid);
> >> @@ -709,6 +774,7 @@ static int __init parse_numa_properties(void)
> >> struct device_node *memory;
> >> int default_nid = 0;
> >> unsigned long i;
> >> + const __be32 *associativity;
> >>
> >> if (numa_enabled == 0) {
> >> printk(KERN_WARNING "NUMA disabled by user\n");
> >> @@ -734,18 +800,30 @@ static int __init parse_numa_properties(void)
> >> * each node to be onlined must have NODE_DATA etc backing it.
> >> */
> >> for_each_present_cpu(i) {
> >> + __be32 vphn_assoc[VPHN_ASSOC_BUFSIZE];
> >> struct device_node *cpu;
> >> - int nid = vphn_get_nid(i);
> >> + int nid = NUMA_NO_NODE;
> >>
> >> - /*
> >> - * Don't fall back to default_nid yet -- we will plug
> >> - * cpus into nodes once the memory scan has discovered
> >> - * the topology.
> >> - */
> >> - if (nid == NUMA_NO_NODE) {
> >> + memset(vphn_assoc, 0, VPHN_ASSOC_BUFSIZE * sizeof(__be32));
> >
> > What's the memset() for? AFAICT you only look at vphn_assoc in the
> > branch where __vphn_get_associativity() succeeds.
>
> That was done to match the existing code. We do use a zero filled array
> when making that hcall in this code path. I don't see us doing that
> everywhere. But didn't want to change that behaviour in this patch.
>
> -static int vphn_get_nid(long lcpu)
> +
> +static int __vphn_get_associativity(long lcpu, __be32 *associativity)
> {
> - __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> long rc, hwid;
Ok, that makes sense.
>
> >
> >> +
> >> + if (__vphn_get_associativity(i, vphn_assoc) == 0) {
> >> + nid = associativity_to_nid(vphn_assoc);
> >> + __initialize_form1_numa_distance(vphn_assoc);
> >> + } else {
> >> +
> >> + /*
> >> + * Don't fall back to default_nid yet -- we will plug
> >> + * cpus into nodes once the memory scan has discovered
> >> + * the topology.
> >> + */
> >> cpu = of_get_cpu_node(i, NULL);
> >> BUG_ON(!cpu);
> >> - nid = of_node_to_nid_single(cpu);
> >> +
> >> + associativity = of_get_associativity(cpu);
> >> + if (associativity) {
> >> + nid = associativity_to_nid(associativity);
> >> + __initialize_form1_numa_distance(associativity);
> >> + }
> >> of_node_put(cpu);
> >> }
> >>
> >> @@ -781,8 +859,11 @@ static int __init parse_numa_properties(void)
> >> * have associativity properties. If none, then
> >> * everything goes to default_nid.
> >> */
> >> - nid = of_node_to_nid_single(memory);
> >> - if (nid < 0)
> >> + associativity = of_get_associativity(memory);
> >> + if (associativity) {
> >> + nid = associativity_to_nid(associativity);
> >> + __initialize_form1_numa_distance(associativity);
> >> + } else
> >> nid = default_nid;
> >>
> >> fake_numa_create_new_node(((start + size) >> PAGE_SHIFT), &nid);
> >> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> >> index 7e970f81d8ff..778b6ab35f0d 100644
> >> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> >> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> >> @@ -498,6 +498,8 @@ static ssize_t dlpar_cpu_add(u32 drc_index)
> >> return saved_rc;
> >> }
> >>
> >> + update_numa_distance(dn);
> >> +
> >> rc = dlpar_online_cpu(dn);
> >> if (rc) {
> >> saved_rc = rc;
> >> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> >> index 36f66556a7c6..40d350f31a34 100644
> >> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> >> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> >> @@ -180,6 +180,8 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
> >> return -ENODEV;
> >> }
> >>
> >> + update_numa_distance(lmb_node);
> >> +
> >> dr_node = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
> >> if (!dr_node) {
> >> dlpar_free_cc_nodes(lmb_node);
> >> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> >> index 1f051a786fb3..663a0859cf13 100644
> >> --- a/arch/powerpc/platforms/pseries/pseries.h
> >> +++ b/arch/powerpc/platforms/pseries/pseries.h
> >> @@ -113,4 +113,5 @@ extern u32 pseries_security_flavor;
> >> void pseries_setup_security_mitigations(void);
> >> void pseries_lpar_read_hblkrm_characteristics(void);
> >>
> >> +void update_numa_distance(struct device_node *node);
> >> #endif /* _PSERIES_PSERIES_H */
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 6/6] powerpc/pseries: Add support for FORM2 associativity
From: David Gibson @ 2021-07-26 2:41 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Nathan Lynch, Daniel Henrique Barboza, linuxppc-dev
In-Reply-To: <87tukmu7hp.fsf@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 22269 bytes --]
On Thu, Jul 22, 2021 at 01:04:42PM +0530, Aneesh Kumar K.V wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
>
> > On Mon, Jun 28, 2021 at 08:41:17PM +0530, Aneesh Kumar K.V wrote:
> >> PAPR interface currently supports two different ways of communicating resource
> >> grouping details to the OS. These are referred to as Form 0 and Form 1
> >> associativity grouping. Form 0 is the older format and is now considered
> >> deprecated. This patch adds another resource grouping named FORM2.
> >>
> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> >> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> >> ---
> >> Documentation/powerpc/associativity.rst | 103 ++++++++++++++
> >> arch/powerpc/include/asm/firmware.h | 3 +-
> >> arch/powerpc/include/asm/prom.h | 1 +
> >> arch/powerpc/kernel/prom_init.c | 3 +-
> >> arch/powerpc/mm/numa.c | 157 ++++++++++++++++++----
> >> arch/powerpc/platforms/pseries/firmware.c | 1 +
> >> 6 files changed, 242 insertions(+), 26 deletions(-)
> >> create mode 100644 Documentation/powerpc/associativity.rst
> >>
> >> diff --git a/Documentation/powerpc/associativity.rst b/Documentation/powerpc/associativity.rst
> >> new file mode 100644
> >> index 000000000000..31cc7da2c7a6
> >> --- /dev/null
> >> +++ b/Documentation/powerpc/associativity.rst
> >> @@ -0,0 +1,103 @@
> >> +============================
> >> +NUMA resource associativity
> >> +=============================
> >> +
> >> +Associativity represents the groupings of the various platform resources into
> >> +domains of substantially similar mean performance relative to resources outside
> >> +of that domain. Resources subsets of a given domain that exhibit better
> >> +performance relative to each other than relative to other resources subsets
> >> +are represented as being members of a sub-grouping domain. This performance
> >> +characteristic is presented in terms of NUMA node distance within the Linux kernel.
> >> +From the platform view, these groups are also referred to as domains.
> >
> > Pretty hard to decipher, but that's typical for PAPR.
> >
> >> +PAPR interface currently supports different ways of communicating these resource
> >> +grouping details to the OS. These are referred to as Form 0, Form 1 and Form2
> >> +associativity grouping. Form 0 is the older format and is now considered deprecated.
> >
> > Nit: s/older/oldest/ since there are now >2 forms.
>
> updated.
>
> >
> >> +Hypervisor indicates the type/form of associativity used via "ibm,architecture-vec-5 property".
> >> +Bit 0 of byte 5 in the "ibm,architecture-vec-5" property indicates usage of Form 0 or Form 1.
> >> +A value of 1 indicates the usage of Form 1 associativity. For Form 2 associativity
> >> +bit 2 of byte 5 in the "ibm,architecture-vec-5" property is used.
> >> +
> >> +Form 0
> >> +-----
> >> +Form 0 associativity supports only two NUMA distances (LOCAL and REMOTE).
> >> +
> >> +Form 1
> >> +-----
> >> +With Form 1 a combination of ibm,associativity-reference-points, and ibm,associativity
> >> +device tree properties are used to determine the NUMA distance between resource groups/domains.
> >> +
> >> +The “ibm,associativity” property contains a list of one or more numbers (domainID)
> >> +representing the resource’s platform grouping domains.
> >> +
> >> +The “ibm,associativity-reference-points” property contains a list of one or more numbers
> >> +(domainID index) that represents the 1 based ordinal in the associativity lists.
> >> +The list of domainID indexes represents an increasing hierarchy of resource grouping.
> >> +
> >> +ex:
> >> +{ primary domainID index, secondary domainID index, tertiary domainID index.. }
> >> +
> >> +Linux kernel uses the domainID at the primary domainID index as the NUMA node id.
> >> +Linux kernel computes NUMA distance between two domains by recursively comparing
> >> +if they belong to the same higher-level domains. For mismatch at every higher
> >> +level of the resource group, the kernel doubles the NUMA distance between the
> >> +comparing domains.
> >> +
> >> +Form 2
> >> +-------
> >> +Form 2 associativity format adds separate device tree properties representing NUMA node distance
> >> +thereby making the node distance computation flexible. Form 2 also allows flexible primary
> >> +domain numbering. With numa distance computation now detached from the index value in
> >> +"ibm,associativity-reference-points" property, Form 2 allows a large number of primary domain
> >> +ids at the same domainID index representing resource groups of different performance/latency
> >> +characteristics.
> >> +
> >> +Hypervisor indicates the usage of FORM2 associativity using bit 2 of byte 5 in the
> >> +"ibm,architecture-vec-5" property.
> >> +
> >> +"ibm,numa-lookup-index-table" property contains a list of one or more numbers representing
> >> +the domainIDs present in the system. The offset of the domainID in this property is
> >> +used as an index while computing numa distance information via "ibm,numa-distance-table".
> >> +
> >> +prop-encoded-array: The number N of the domainIDs encoded as with encode-int, followed by
> >> +N domainID encoded as with encode-int
> >> +
> >> +For ex:
> >> +"ibm,numa-lookup-index-table" = {4, 0, 8, 250, 252}. The offset of domainID 8 (2) is used when
> >> +computing the distance of domain 8 from other domains present in the system. For the rest of
> >> +this document, this offset will be referred to as domain distance offset.
> >> +
> >> +"ibm,numa-distance-table" property contains a list of one or more numbers representing the NUMA
> >> +distance between resource groups/domains present in the system.
> >> +
> >> +prop-encoded-array: The number N of the distance values encoded as with encode-int, followed by
> >> +N distance values encoded as with encode-bytes. The max distance value we could encode is 255.
> >> +The number N must be equal to the square of m where m is the number of domainIDs in the
> >> +numa-lookup-index-table.
> >> +
> >> +For ex:
> >> +ibm,numa-lookup-index-table = {3, 0, 8, 40}
> >> +ibm,numa-distance-table = {9, 10, 20, 80, 20, 10, 160, 80, 160, 10}
> >
> > This representation doesn't make it clear that the 9 is a u32, but the
> > rest are u8s.
>
> How do you suggest we specify that? I could do 9:u32 10:u8 etc. But
> considering the details are explained in the paragraph above, is that
> needed?
Yes, I think it is needed. The examples are, honestly, a lot easier
to read and follow than the PAPR-ese text, so people are much more
likely to be looking at those than parsing the minutiae of the text.
> >> +
> >> + | 0 8 40
> >> +--|------------
> >> + |
> >> +0 | 10 20 80
> >> + |
> >> +8 | 20 10 160
> >> + |
> >> +40| 80 160 10
> >> +
> >> +A possible "ibm,associativity" property for resources in node 0, 8 and 40
> >> +
> >> +{ 3, 6, 7, 0 }
> >> +{ 3, 6, 9, 8 }
> >> +{ 3, 6, 7, 40}
> >> +
> >> +With "ibm,associativity-reference-points" { 0x3 }
> >
> > You haven't actually described how ibm,associativity-reference-points
> > operates in Form2.
>
> Nothing change w.r.t the definition of associativity-reference-points
> w.r.t FORM2. It still will continue to show the increasing hierarchy of
> resource groups.
I guess, except that really none of them matter except the primary any
more.
>
> >
> >> +"ibm,lookup-index-table" helps in having a compact representation of distance matrix.
> >> +Since domainID can be sparse, the matrix of distances can also be effectively sparse.
> >> +With "ibm,lookup-index-table" we can achieve a compact representation of
> >> +distance information.
> >> diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
> >> index 60b631161360..97a3bd9ffeb9 100644
> >> --- a/arch/powerpc/include/asm/firmware.h
> >> +++ b/arch/powerpc/include/asm/firmware.h
> >> @@ -53,6 +53,7 @@
> >> #define FW_FEATURE_ULTRAVISOR ASM_CONST(0x0000004000000000)
> >> #define FW_FEATURE_STUFF_TCE ASM_CONST(0x0000008000000000)
> >> #define FW_FEATURE_RPT_INVALIDATE ASM_CONST(0x0000010000000000)
> >> +#define FW_FEATURE_FORM2_AFFINITY ASM_CONST(0x0000020000000000)
> >>
> >> #ifndef __ASSEMBLY__
> >>
> >> @@ -73,7 +74,7 @@ enum {
> >> FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
> >> FW_FEATURE_DRC_INFO | FW_FEATURE_BLOCK_REMOVE |
> >> FW_FEATURE_PAPR_SCM | FW_FEATURE_ULTRAVISOR |
> >> - FW_FEATURE_RPT_INVALIDATE,
> >> + FW_FEATURE_RPT_INVALIDATE | FW_FEATURE_FORM2_AFFINITY,
> >> FW_FEATURE_PSERIES_ALWAYS = 0,
> >> FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL | FW_FEATURE_ULTRAVISOR,
> >> FW_FEATURE_POWERNV_ALWAYS = 0,
> >> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
> >> index df9fec9d232c..5c80152e8f18 100644
> >> --- a/arch/powerpc/include/asm/prom.h
> >> +++ b/arch/powerpc/include/asm/prom.h
> >> @@ -149,6 +149,7 @@ extern int of_read_drc_info_cell(struct property **prop,
> >> #define OV5_XCMO 0x0440 /* Page Coalescing */
> >> #define OV5_FORM1_AFFINITY 0x0580 /* FORM1 NUMA affinity */
> >> #define OV5_PRRN 0x0540 /* Platform Resource Reassignment */
> >> +#define OV5_FORM2_AFFINITY 0x0520 /* Form2 NUMA affinity */
> >> #define OV5_HP_EVT 0x0604 /* Hot Plug Event support */
> >> #define OV5_RESIZE_HPT 0x0601 /* Hash Page Table resizing */
> >> #define OV5_PFO_HW_RNG 0x1180 /* PFO Random Number Generator */
> >> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> >> index 5d9ea059594f..c483df6c9393 100644
> >> --- a/arch/powerpc/kernel/prom_init.c
> >> +++ b/arch/powerpc/kernel/prom_init.c
> >> @@ -1069,7 +1069,8 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
> >> #else
> >> 0,
> >> #endif
> >> - .associativity = OV5_FEAT(OV5_FORM1_AFFINITY) | OV5_FEAT(OV5_PRRN),
> >> + .associativity = OV5_FEAT(OV5_FORM1_AFFINITY) | OV5_FEAT(OV5_PRRN) |
> >> + OV5_FEAT(OV5_FORM2_AFFINITY),
> >> .bin_opts = OV5_FEAT(OV5_RESIZE_HPT) | OV5_FEAT(OV5_HP_EVT),
> >> .micro_checkpoint = 0,
> >> .reserved0 = 0,
> >> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> >> index c6293037a103..c68846fc9550 100644
> >> --- a/arch/powerpc/mm/numa.c
> >> +++ b/arch/powerpc/mm/numa.c
> >> @@ -56,12 +56,17 @@ static int n_mem_addr_cells, n_mem_size_cells;
> >>
> >> #define FORM0_AFFINITY 0
> >> #define FORM1_AFFINITY 1
> >> +#define FORM2_AFFINITY 2
> >> static int affinity_form;
> >>
> >> #define MAX_DISTANCE_REF_POINTS 4
> >> static int max_associativity_domain_index;
> >> static const __be32 *distance_ref_points;
> >> static int distance_lookup_table[MAX_NUMNODES][MAX_DISTANCE_REF_POINTS];
> >> +static int numa_distance_table[MAX_NUMNODES][MAX_NUMNODES] = {
> >> + [0 ... MAX_NUMNODES - 1] = { [0 ... MAX_NUMNODES - 1] = -1 }
> >> +};
> >> +static int numa_id_index_table[MAX_NUMNODES] = { [0 ... MAX_NUMNODES - 1] = NUMA_NO_NODE };
> >>
> >> /*
> >> * Allocate node_to_cpumask_map based on number of available nodes
> >> @@ -166,6 +171,44 @@ static void unmap_cpu_from_node(unsigned long cpu)
> >> }
> >> #endif /* CONFIG_HOTPLUG_CPU || CONFIG_PPC_SPLPAR */
> >>
> >> +/*
> >> + * Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
> >> + * info is found.
> >> + */
> >> +static int associativity_to_nid(const __be32 *associativity)
> >> +{
> >> + int nid = NUMA_NO_NODE;
> >> +
> >> + if (!numa_enabled)
> >> + goto out;
> >> +
> >> + if (of_read_number(associativity, 1) >= primary_domain_index)
> >> + nid = of_read_number(&associativity[primary_domain_index], 1);
> >> +
> >> + /* POWER4 LPAR uses 0xffff as invalid node */
> >> + if (nid == 0xffff || nid >= nr_node_ids)
> >> + nid = NUMA_NO_NODE;
> >> +out:
> >> + return nid;
> >> +}
> >> +
> >> +static int __cpu_form2_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> +{
> >> + int dist;
> >> + int node1, node2;
> >> +
> >> + node1 = associativity_to_nid(cpu1_assoc);
> >> + node2 = associativity_to_nid(cpu2_assoc);
> >> +
> >> + dist = numa_distance_table[node1][node2];
> >> + if (dist <= LOCAL_DISTANCE)
> >> + return 0;
> >> + else if (dist <= REMOTE_DISTANCE)
> >> + return 1;
> >> + else
> >> + return 2;
> >
> > Squashing the full range of distances into just 0, 1 or 2 seems odd.
> > But then, this whole cpu_distance() thing being distinct from
> > node_distance() seems odd.
> >
> >> +}
> >> +
> >> static int __cpu_form1_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> {
> >> int dist = 0;
> >> @@ -186,8 +229,9 @@ int cpu_relative_distance(__be32 *cpu1_assoc, __be32 *cpu2_assoc)
> >> {
> >> /* We should not get called with FORM0 */
> >> VM_WARN_ON(affinity_form == FORM0_AFFINITY);
> >> -
> >> - return __cpu_form1_relative_distance(cpu1_assoc, cpu2_assoc);
> >> + if (affinity_form == FORM1_AFFINITY)
> >> + return __cpu_form1_relative_distance(cpu1_assoc, cpu2_assoc);
> >> + return __cpu_form2_relative_distance(cpu1_assoc, cpu2_assoc);
> >> }
> >>
> >> /* must hold reference to node during call */
> >> @@ -201,7 +245,9 @@ int __node_distance(int a, int b)
> >> int i;
> >> int distance = LOCAL_DISTANCE;
> >>
> >> - if (affinity_form == FORM0_AFFINITY)
> >> + if (affinity_form == FORM2_AFFINITY)
> >> + return numa_distance_table[a][b];
> >> + else if (affinity_form == FORM0_AFFINITY)
> >> return ((a == b) ? LOCAL_DISTANCE : REMOTE_DISTANCE);
> >>
> >> for (i = 0; i < max_associativity_domain_index; i++) {
> >
> > Hmm.. couldn't we simplify this whole __node_distance function, if we
> > just update numa_distance_table[][] appropriately for Form0 and Form1
> > as well?
>
> IIUC what you are suggesting is to look at the possibility of using
> numa_distance_table[a][b] even for FORM1_AFFINITY? I can do that as part
> of separate patch?
Ok, that's reasonable.
> >
> >> @@ -216,27 +262,6 @@ int __node_distance(int a, int b)
> >> }
> >> EXPORT_SYMBOL(__node_distance);
> >>
> >> -/*
> >> - * Returns nid in the range [0..nr_node_ids], or -1 if no useful NUMA
> >> - * info is found.
> >> - */
> >> -static int associativity_to_nid(const __be32 *associativity)
> >> -{
> >> - int nid = NUMA_NO_NODE;
> >> -
> >> - if (!numa_enabled)
> >> - goto out;
> >> -
> >> - if (of_read_number(associativity, 1) >= primary_domain_index)
> >> - nid = of_read_number(&associativity[primary_domain_index], 1);
> >> -
> >> - /* POWER4 LPAR uses 0xffff as invalid node */
> >> - if (nid == 0xffff || nid >= nr_node_ids)
> >> - nid = NUMA_NO_NODE;
> >> -out:
> >> - return nid;
> >> -}
> >> -
> >> /* Returns the nid associated with the given device tree node,
> >> * or -1 if not found.
> >> */
> >> @@ -305,12 +330,84 @@ static void initialize_form1_numa_distance(struct device_node *node)
> >> */
> >> void update_numa_distance(struct device_node *node)
> >> {
> >> + int nid;
> >> +
> >> if (affinity_form == FORM0_AFFINITY)
> >> return;
> >> else if (affinity_form == FORM1_AFFINITY) {
> >> initialize_form1_numa_distance(node);
> >> return;
> >> }
> >> +
> >> + /* FORM2 affinity */
> >> + nid = of_node_to_nid_single(node);
> >> + if (nid == NUMA_NO_NODE)
> >> + return;
> >> +
> >> + /*
> >> + * With FORM2 we expect NUMA distance of all possible NUMA
> >> + * nodes to be provided during boot.
> >> + */
> >> + WARN(numa_distance_table[nid][nid] == -1,
> >> + "NUMA distance details for node %d not provided\n", nid);
> >> +}
> >> +
> >> +/*
> >> + * ibm,numa-lookup-index-table= {N, domainid1, domainid2, ..... domainidN}
> >> + * ibm,numa-distance-table = { N, 1, 2, 4, 5, 1, 6, .... N elements}
> >> + */
> >> +static void initialize_form2_numa_distance_lookup_table(struct device_node *root)
> >> +{
> >> + int i, j;
> >> + const __u8 *numa_dist_table;
> >> + const __be32 *numa_lookup_index;
> >> + int numa_dist_table_length;
> >> + int max_numa_index, distance_index;
> >> +
> >> + numa_lookup_index = of_get_property(root, "ibm,numa-lookup-index-table", NULL);
> >> + max_numa_index = of_read_number(&numa_lookup_index[0], 1);
> >> +
> >> + /* first element of the array is the size and is encode-int */
> >> + numa_dist_table = of_get_property(root, "ibm,numa-distance-table", NULL);
> >> + numa_dist_table_length = of_read_number((const __be32 *)&numa_dist_table[0], 1);
> >> + /* Skip the size which is encoded int */
> >> + numa_dist_table += sizeof(__be32);
> >> +
> >> + pr_debug("numa_dist_table_len = %d, numa_dist_indexes_len = %d\n",
> >> + numa_dist_table_length, max_numa_index);
> >> +
> >> + for (i = 0; i < max_numa_index; i++)
> >> + /* +1 skip the max_numa_index in the property */
> >> + numa_id_index_table[i] = of_read_number(&numa_lookup_index[i + 1], 1);
> >> +
> >> +
> >> + if (numa_dist_table_length != max_numa_index * max_numa_index) {
> >> +
> >> + WARN(1, "Wrong NUMA distance information\n");
> >> + /* consider everybody else just remote. */
> >> + for (i = 0; i < max_numa_index; i++) {
> >> + for (j = 0; j < max_numa_index; j++) {
> >> + int nodeA = numa_id_index_table[i];
> >> + int nodeB = numa_id_index_table[j];
> >> +
> >> + if (nodeA == nodeB)
> >> + numa_distance_table[nodeA][nodeB] = LOCAL_DISTANCE;
> >> + else
> >> + numa_distance_table[nodeA][nodeB] = REMOTE_DISTANCE;
> >> + }
> >> + }
> >> + }
> >> +
> >> + distance_index = 0;
> >> + for (i = 0; i < max_numa_index; i++) {
> >> + for (j = 0; j < max_numa_index; j++) {
> >> + int nodeA = numa_id_index_table[i];
> >> + int nodeB = numa_id_index_table[j];
> >> +
> >> + numa_distance_table[nodeA][nodeB] = numa_dist_table[distance_index++];
> >> + pr_debug("dist[%d][%d]=%d ", nodeA, nodeB, numa_distance_table[nodeA][nodeB]);
> >> + }
> >> + }
> >> }
> >>
> >> static int __init find_primary_domain_index(void)
> >> @@ -323,6 +420,9 @@ static int __init find_primary_domain_index(void)
> >> */
> >> if (firmware_has_feature(FW_FEATURE_OPAL)) {
> >> affinity_form = FORM1_AFFINITY;
> >> + } else if (firmware_has_feature(FW_FEATURE_FORM2_AFFINITY)) {
> >> + dbg("Using form 2 affinity\n");
> >> + affinity_form = FORM2_AFFINITY;
> >> } else if (firmware_has_feature(FW_FEATURE_FORM1_AFFINITY)) {
> >> dbg("Using form 1 affinity\n");
> >> affinity_form = FORM1_AFFINITY;
> >> @@ -367,8 +467,17 @@ static int __init find_primary_domain_index(void)
> >>
> >> index = of_read_number(&distance_ref_points[1], 1);
> >> } else {
> >> + /*
> >> + * Both FORM1 and FORM2 affinity find the primary domain details
> >> + * at the same offset.
> >> + */
> >> index = of_read_number(distance_ref_points, 1);
> >> }
> >> + /*
> >> + * If it is FORM2 also initialize the distance table here.
> >> + */
> >> + if (affinity_form == FORM2_AFFINITY)
> >> + initialize_form2_numa_distance_lookup_table(root);
> >
> > Ew. Calling a function called "find_primary_domain_index" to also
> > initialize the main distance table is needlessly counterintuitive.
> > Move this call to parse_numa_properties().
>
> The reason I ended up doing it here is because 'root' is already fetched
> here. But I agree it is confusing. I will move fetching of root inside
> initialize_form2_numa_distance_lookup_table() and move the function
> outside primary_index lookup.
Ok. This is not a hot path anyway, so looking up root twice isn't
really a big deal anyway.
>
> modified arch/powerpc/mm/numa.c
> @@ -355,14 +355,22 @@ void update_numa_distance(struct device_node *node)
> * ibm,numa-lookup-index-table= {N, domainid1, domainid2, ..... domainidN}
> * ibm,numa-distance-table = { N, 1, 2, 4, 5, 1, 6, .... N elements}
> */
> -static void initialize_form2_numa_distance_lookup_table(struct device_node *root)
> +static void initialize_form2_numa_distance_lookup_table()
> {
> int i, j;
> + struct device_node *root;
> const __u8 *numa_dist_table;
> const __be32 *numa_lookup_index;
> int numa_dist_table_length;
> int max_numa_index, distance_index;
>
> + if (firmware_has_feature(FW_FEATURE_OPAL))
> + root = of_find_node_by_path("/ibm,opal");
> + else
> + root = of_find_node_by_path("/rtas");
> + if (!root)
> + root = of_find_node_by_path("/");
> +
> numa_lookup_index = of_get_property(root, "ibm,numa-lookup-index-table", NULL);
> max_numa_index = of_read_number(&numa_lookup_index[0], 1);
>
> @@ -407,6 +415,7 @@ static void initialize_form2_numa_distance_lookup_table(struct device_node *root
> pr_debug("dist[%d][%d]=%d ", nodeA, nodeB, numa_distance_table[nodeA][nodeB]);
> }
> }
> + of_node_put(root);
> }
>
> static int __init find_primary_domain_index(void)
> @@ -472,12 +481,6 @@ static int __init find_primary_domain_index(void)
> */
> index = of_read_number(distance_ref_points, 1);
> }
> - /*
> - * If it is FORM2 also initialize the distance table here.
> - */
> - if (affinity_form == FORM2_AFFINITY)
> - initialize_form2_numa_distance_lookup_table(root);
> -
> /*
> * Warn and cap if the hardware supports more than
> * MAX_DISTANCE_REF_POINTS domains.
> @@ -916,6 +919,12 @@ static int __init parse_numa_properties(void)
>
> dbg("NUMA associativity depth for CPU/Memory: %d\n", primary_domain_index);
>
> + /*
> + * If it is FORM2 also initialize the distance table here.
> + */
> + if (affinity_form == FORM2_AFFINITY)
> + initialize_form2_numa_distance_lookup_table();
> +
> /*
> * Even though we connect cpus to numa domains later in SMP
> * init, we need to know the node ids now. This is because
>
> -aneesh
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH 1/2] powerpc/64s: POWER9 DD2.3 CPU feature flag fixes
From: Nicholas Piggin @ 2021-07-26 3:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
DD2.3 missed out on getting its feature flag bits.
This meant when booting with dt-cpu-ftrs, CPU_FTR_P9_TM_HV_ASSIST is
missing (unless the firmware contains it, which mine does not seem to).
And when booting without, CPU_FTR_P9_TM_XER_SO_BUG is set.
In practice this doesn't make any difference to pseries guests, only
powernv.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/cputable.h | 2 ++
arch/powerpc/kernel/cputable.c | 22 ++++++++++++++++++++--
arch/powerpc/kernel/dt_cpu_ftrs.c | 14 +++++---------
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index e85c849214a2..46bae9624784 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -440,6 +440,8 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
CPU_FTR_P9_TM_HV_ASSIST | \
CPU_FTR_P9_TM_XER_SO_BUG)
+#define CPU_FTRS_POWER9_DD2_3 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
+ CPU_FTR_P9_TM_HV_ASSIST)
#define CPU_FTRS_POWER10 (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
CPU_FTR_MMCRA | CPU_FTR_SMT | \
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index ae0fdef0ac11..9ab97d1fd5a2 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -487,11 +487,29 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check_early = __machine_check_early_realmode_p9,
.platform = "power9",
},
- { /* Power9 DD2.2 or later */
+ { /* Power9 DD 2.2 */
+ .pvr_mask = 0xffffefff,
+ .pvr_value = 0x004e0202,
+ .cpu_name = "POWER9 (raw)",
+ .cpu_features = CPU_FTRS_POWER9_DD2_2,
+ .cpu_user_features = COMMON_USER_POWER9,
+ .cpu_user_features2 = COMMON_USER2_POWER9,
+ .mmu_features = MMU_FTRS_POWER9,
+ .icache_bsize = 128,
+ .dcache_bsize = 128,
+ .num_pmcs = 6,
+ .pmc_type = PPC_PMC_IBM,
+ .oprofile_cpu_type = "ppc64/power9",
+ .cpu_setup = __setup_cpu_power9,
+ .cpu_restore = __restore_cpu_power9,
+ .machine_check_early = __machine_check_early_realmode_p9,
+ .platform = "power9",
+ },
+ { /* Power9 DD 2.3 or later */
.pvr_mask = 0xffff0000,
.pvr_value = 0x004e0000,
.cpu_name = "POWER9 (raw)",
- .cpu_features = CPU_FTRS_POWER9_DD2_2,
+ .cpu_features = CPU_FTRS_POWER9_DD2_3,
.cpu_user_features = COMMON_USER_POWER9,
.cpu_user_features2 = COMMON_USER2_POWER9,
.mmu_features = MMU_FTRS_POWER9,
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index 358aee7c2d79..af95f337e54b 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -764,18 +764,14 @@ static __init void cpufeatures_cpu_quirks(void)
* Not all quirks can be derived from the cpufeatures device tree.
*/
if ((version & 0xffffefff) == 0x004e0200) {
- /* DD2.0 has no feature flag */
- cur_cpu_spec->cpu_features |= CPU_FTR_P9_RADIX_PREFETCH_BUG;
+ cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_0;
} else if ((version & 0xffffefff) == 0x004e0201) {
- cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
- cur_cpu_spec->cpu_features |= CPU_FTR_P9_RADIX_PREFETCH_BUG;
+ cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_1;
} else if ((version & 0xffffefff) == 0x004e0202) {
- cur_cpu_spec->cpu_features |= CPU_FTR_P9_TM_HV_ASSIST;
- cur_cpu_spec->cpu_features |= CPU_FTR_P9_TM_XER_SO_BUG;
- cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
+ cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_2;
} else if ((version & 0xffff0000) == 0x004e0000) {
- /* DD2.1 and up have DD2_1 */
- cur_cpu_spec->cpu_features |= CPU_FTR_POWER9_DD2_1;
+ /* DD2.3 and up */
+ cur_cpu_spec->cpu_features |= CPU_FTRS_POWER9_DD2_3;
}
if ((version & 0xffff0000) == 0x004e0000) {
--
2.23.0
^ permalink raw reply related
* [PATCH 2/2] powerpc/64s: Rename CPU_FTR_POWER9_DD2_1 to CPU_FTR_P9_STOP_FIXED
From: Nicholas Piggin @ 2021-07-26 3:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20210726031730.729934-1-npiggin@gmail.com>
CPU feature flags work best when they are named for behaviour, not for
the CPU variant that first introduced them. Later revisions might also
contain the behaviour, for example. It's confusing for a POWER9 DD2.2
to have CPU_FTR_POWER9_DD2_1, but it's not confusing if DD2.1 and DD2.2
both have CPU_FTR_P9_STOP_FIXED.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/cputable.h | 8 ++++----
arch/powerpc/platforms/powernv/idle.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 46bae9624784..cb9948f318f7 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -186,7 +186,7 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTR_DAWR LONG_ASM_CONST(0x0000008000000000)
#define CPU_FTR_DABRX LONG_ASM_CONST(0x0000010000000000)
#define CPU_FTR_PMAO_BUG LONG_ASM_CONST(0x0000020000000000)
-#define CPU_FTR_POWER9_DD2_1 LONG_ASM_CONST(0x0000080000000000)
+#define CPU_FTR_P9_STOP_FIXED LONG_ASM_CONST(0x0000080000000000)
#define CPU_FTR_P9_TM_HV_ASSIST LONG_ASM_CONST(0x0000100000000000)
#define CPU_FTR_P9_TM_XER_SO_BUG LONG_ASM_CONST(0x0000200000000000)
#define CPU_FTR_P9_TLBIE_STQ_BUG LONG_ASM_CONST(0x0000400000000000)
@@ -436,11 +436,11 @@ static inline void cpu_feature_keys_init(void) { }
#define CPU_FTRS_POWER9_DD2_0 (CPU_FTRS_POWER9 | CPU_FTR_P9_RADIX_PREFETCH_BUG)
#define CPU_FTRS_POWER9_DD2_1 (CPU_FTRS_POWER9 | \
CPU_FTR_P9_RADIX_PREFETCH_BUG | \
- CPU_FTR_POWER9_DD2_1)
-#define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
+ CPU_FTR_P9_STOP_FIXED)
+#define CPU_FTRS_POWER9_DD2_2 (CPU_FTRS_POWER9 | CPU_FTR_P9_STOP_FIXED | \
CPU_FTR_P9_TM_HV_ASSIST | \
CPU_FTR_P9_TM_XER_SO_BUG)
-#define CPU_FTRS_POWER9_DD2_3 (CPU_FTRS_POWER9 | CPU_FTR_POWER9_DD2_1 | \
+#define CPU_FTRS_POWER9_DD2_3 (CPU_FTRS_POWER9 | CPU_FTR_P9_STOP_FIXED | \
CPU_FTR_P9_TM_HV_ASSIST)
#define CPU_FTRS_POWER10 (CPU_FTR_LWSYNC | \
CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 528a7e0cf83a..1e908536890b 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -650,7 +650,7 @@ static unsigned long power9_idle_stop(unsigned long psscr)
}
#endif
- if (!cpu_has_feature(CPU_FTR_POWER9_DD2_1)) {
+ if (!cpu_has_feature(CPU_FTR_P9_STOP_FIXED)) {
/*
* POWER9 DD2 can incorrectly set PMAO when waking up
* after a state-loss idle. Saving and restoring MMCR0
@@ -717,7 +717,7 @@ static unsigned long power9_idle_stop(unsigned long psscr)
* might have been corrupted and needs flushing. We also need
* to reload MMCR0 (see mmcr0 comment above).
*/
- if (!cpu_has_feature(CPU_FTR_POWER9_DD2_1)) {
+ if (!cpu_has_feature(CPU_FTR_P9_STOP_FIXED)) {
asm volatile(PPC_ISA_3_0_INVALIDATE_ERAT);
mtspr(SPRN_MMCR0, mmcr0);
}
--
2.23.0
^ permalink raw reply related
* [PATCH v1 00/55] KVM: PPC: Book3S HV P9: entry/exit optimisations
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
This reduces radix guest full entry/exit latency on POWER9 and POWER10
by almost 2x.
Nested HV guests should see smaller improvements in their L1 entry/exit,
but this is also combined with most L0 speedups also applying to nested
entry. nginx localhost throughput test in a SMP nested guest is improved
about 10% (in a direct guest it doesn't change much because it uses XIVE
for IPIs) when L0 and L1 are patched.
It does this in several main ways:
- Rearrange code to optimise SPR accesses. Mainly, avoid scoreboard
stalls.
- Test SPR values to avoid mtSPRs where possible. mtSPRs are expensive.
- Reduce mftb. mftb is expensive.
- Demand fault certain facilities to avoid saving and/or restoring them
(at the cost of fault when they are used, but this is mitigated over
a number of entries, like the facilities when context switching
processes). PM, TM, and EBB so far.
- Defer some sequences that are made just in case a guest is interrupted
in the middle of a critical section to the case where the guest is
scheduled on a different CPU, rather than every time (at the cost of
an extra IPI in this case). Namely the tlbsync sequence for radix with
GTSE, which is very expensive.
This also adds the 2nd round patches to the series, which improve
performance mostly by reducing locking, barriers, and atomics related
to the vcpus-per-vcore > 1 handling that the P9 path does not require.
Some of the numbers quoted in changelogs may have changed a bit with
patches being updated, reordered, etc. They give a bit of a guide, but
I might remove them from the final submission because they're too much
to maintain.
Changes since RFC:
- Rebased with Fabiano's HV sanitising patches at the front.
- Several demand faulting bug fixes mostly relating to nested guests.
- Removed facility demand-faulting from L0 nested entry/exit handler.
Demand faulting is still done in the L1, but not the L0. The reason
is to reduce complexity (although it's only a small amount of
complexity), reduce demand faulting overhead that may require several
interrupts, and allow better testing of the L1 demand faulting,
because we may run on hypervisors that do not implement L0 demand
faulting. In future, depending on performance and such, we could add
demand faulting to L0 nested entry handling and/or remove it from the
L1.
- Fixed a timebase problem with the HMI subcore patch.
Fabiano Rosas (2):
KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
KVM: PPC: Book3S HV: Stop forwarding all HFUs to L1
Nicholas Piggin (53):
KVM: PPC: Book3S HV: Remove TM emulation from POWER7/8 path
KVM: PPC: Book3S HV P9: Fixes for TM softpatch interrupt
KVM: PPC: Book3S HV Nested: Reflect guest PMU in-use to L0 when guest
SPRs are live
powerpc/64s: Remove WORT SPR from POWER9/10
KMV: PPC: Book3S HV P9: Use set_dec to set decrementer to host
KVM: PPC: Book3S HV P9: Use host timer accounting to avoid decrementer
read
KVM: PPC: Book3S HV P9: Use large decrementer for HDEC
KVM: PPC: Book3S HV P9: Reduce mftb per guest entry/exit
powerpc/time: add API for KVM to re-arm the host timer/decrementer
KVM: PPC: Book3S HV: POWER10 enable HAIL when running radix guests
powerpc/64s: Keep AMOR SPR a constant ~0 at runtime
KVM: PPC: Book3S HV: Don't always save PMU for guest capable of
nesting
powerpc/64s: Always set PMU control registers to frozen/disabled when
not in use
powerpc/64s: Implement PMU override command line option
KVM: PPC: Book3S HV P9: Implement PMU save/restore in C
KVM: PPC: Book3S HV P9: Factor PMU save/load into context switch
functions
KVM: PPC: Book3S HV P9: Demand fault PMU SPRs when marked not inuse
KVM: PPC: Book3S HV P9: Factor out yield_count increment
KVM: PPC: Book3S HV: CTRL SPR does not require read-modify-write
KVM: PPC: Book3S HV P9: Move SPRG restore to restore_p9_host_os_sprs
KVM: PPC: Book3S HV P9: Reduce mtmsrd instructions required to save
host SPRs
KVM: PPC: Book3S HV P9: Improve mtmsrd scheduling by delaying MSR[EE]
disable
KVM: PPC: Book3S HV P9: Add kvmppc_stop_thread to match
kvmppc_start_thread
KVM: PPC: Book3S HV: Change dec_expires to be relative to guest
timebase
KVM: PPC: Book3S HV P9: Move TB updates
KVM: PPC: Book3S HV P9: Optimise timebase reads
KVM: PPC: Book3S HV P9: Avoid SPR scoreboard stalls
KVM: PPC: Book3S HV P9: Only execute mtSPR if the value changed
KVM: PPC: Book3S HV P9: Juggle SPR switching around
KVM: PPC: Book3S HV P9: Move vcpu register save/restore into functions
KVM: PPC: Book3S HV P9: Move host OS save/restore functions to
built-in
KVM: PPC: Book3S HV P9: Move nested guest entry into its own function
KVM: PPC: Book3S HV P9: Move remaining SPR and MSR access into low
level entry
KVM: PPC: Book3S HV P9: Implement TM fastpath for guest entry/exit
KVM: PPC: Book3S HV P9: Switch PMU to guest as late as possible
KVM: PPC: Book3S HV P9: Restrict DSISR canary workaround to processors
that require it
KVM: PPC: Book3S HV P9: More SPR speed improvements
KVM: PPC: Book3S HV P9: Demand fault EBB facility registers
KVM: PPC: Book3S HV P9: Demand fault TM facility registers
KVM: PPC: Book3S HV P9: Use Linux SPR save/restore to manage some host
SPRs
KVM: PPC: Book3S HV P9: Comment and fix MMU context switching code
KVM: PPC: Book3S HV P9: Test dawr_enabled() before saving host DAWR
SPRs
KVM: PPC: Book3S HV P9: Don't restore PSSCR if not needed
KVM: PPC: Book3S HV P9: Avoid tlbsync sequence on radix guest exit
KVM: PPC: Book3S HV Nested: Avoid extra mftb() in nested entry
KVM: PPC: Book3S HV P9: Improve mfmsr performance on entry
KVM: PPC: Book3S HV P9: Optimise hash guest SLB saving
KVM: PPC: Book3S HV P9: Add unlikely annotation for !mmu_ready
KVM: PPC: Book3S HV P9: Avoid cpu_in_guest atomics on entry and exit
KVM: PPC: Book3S HV P9: Remove most of the vcore logic
KVM: PPC: Book3S HV P9: Tidy kvmppc_create_dtl_entry
KVM: PPC: Book3S HV P9: Stop using vc->dpdes
KVM: PPC: Book3S HV P9: Remove subcore HMI handling
.../admin-guide/kernel-parameters.txt | 7 +
arch/powerpc/include/asm/asm-prototypes.h | 5 -
arch/powerpc/include/asm/kvm_asm.h | 1 +
arch/powerpc/include/asm/kvm_book3s.h | 6 +
arch/powerpc/include/asm/kvm_book3s_64.h | 6 +-
arch/powerpc/include/asm/kvm_host.h | 6 +-
arch/powerpc/include/asm/pmc.h | 7 +
arch/powerpc/include/asm/reg.h | 3 +-
arch/powerpc/include/asm/switch_to.h | 2 +
arch/powerpc/include/asm/time.h | 19 +-
arch/powerpc/kernel/cpu_setup_power.c | 12 +-
arch/powerpc/kernel/dt_cpu_ftrs.c | 8 +-
arch/powerpc/kernel/process.c | 30 +
arch/powerpc/kernel/time.c | 54 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 4 +
arch/powerpc/kvm/book3s_hv.c | 857 ++++++++++--------
arch/powerpc/kvm/book3s_hv.h | 36 +
arch/powerpc/kvm/book3s_hv_builtin.c | 2 +
arch/powerpc/kvm/book3s_hv_hmi.c | 7 +-
arch/powerpc/kvm/book3s_hv_interrupts.S | 13 +-
arch/powerpc/kvm/book3s_hv_nested.c | 131 +--
arch/powerpc/kvm/book3s_hv_p9_entry.c | 793 ++++++++++++++--
arch/powerpc/kvm/book3s_hv_ras.c | 4 +
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 116 +--
arch/powerpc/kvm/book3s_hv_tm.c | 57 +-
arch/powerpc/mm/book3s64/radix_pgtable.c | 15 -
arch/powerpc/perf/core-book3s.c | 35 +
arch/powerpc/platforms/powernv/idle.c | 10 +-
28 files changed, 1508 insertions(+), 738 deletions(-)
create mode 100644 arch/powerpc/kvm/book3s_hv.h
--
2.23.0
^ permalink raw reply
* [PATCH v1 01/55] KVM: PPC: Book3S HV: Remove TM emulation from POWER7/8 path
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
TM fake-suspend emulation is only used by POWER9. Remove it from the old
code path.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 42 -------------------------
1 file changed, 42 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 8dd437d7a2c6..75079397c2a5 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1088,12 +1088,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
cmpwi r12, BOOK3S_INTERRUPT_H_INST_STORAGE
beq kvmppc_hisi
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
- /* For softpatch interrupt, go off and do TM instruction emulation */
- cmpwi r12, BOOK3S_INTERRUPT_HV_SOFTPATCH
- beq kvmppc_tm_emul
-#endif
-
/* See if this is a leftover HDEC interrupt */
cmpwi r12,BOOK3S_INTERRUPT_HV_DECREMENTER
bne 2f
@@ -1599,42 +1593,6 @@ maybe_reenter_guest:
blt deliver_guest_interrupt
b guest_exit_cont
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-/*
- * Softpatch interrupt for transactional memory emulation cases
- * on POWER9 DD2.2. This is early in the guest exit path - we
- * haven't saved registers or done a treclaim yet.
- */
-kvmppc_tm_emul:
- /* Save instruction image in HEIR */
- mfspr r3, SPRN_HEIR
- stw r3, VCPU_HEIR(r9)
-
- /*
- * The cases we want to handle here are those where the guest
- * is in real suspend mode and is trying to transition to
- * transactional mode.
- */
- lbz r0, HSTATE_FAKE_SUSPEND(r13)
- cmpwi r0, 0 /* keep exiting guest if in fake suspend */
- bne guest_exit_cont
- rldicl r3, r11, 64 - MSR_TS_S_LG, 62
- cmpwi r3, 1 /* or if not in suspend state */
- bne guest_exit_cont
-
- /* Call C code to do the emulation */
- mr r3, r9
- bl kvmhv_p9_tm_emulation_early
- nop
- ld r9, HSTATE_KVM_VCPU(r13)
- li r12, BOOK3S_INTERRUPT_HV_SOFTPATCH
- cmpwi r3, 0
- beq guest_exit_cont /* continue exiting if not handled */
- ld r10, VCPU_PC(r9)
- ld r11, VCPU_MSR(r9)
- b fast_interrupt_c_return /* go back to guest if handled */
-#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
-
/*
* Check whether an HDSI is an HPTE not found fault or something else.
* If it is an HPTE not found fault that is due to the guest accessing
--
2.23.0
^ permalink raw reply related
* [PATCH v1 02/55] KVM: PPC: Book3S HV P9: Fixes for TM softpatch interrupt
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
The softpatch interrupt sets HSRR0 to the faulting instruction +4, so
it should subtract 4 for the faulting instruction address. Also have it
emulate and deliver HFAC interrupts correctly, which is important for
nested HV and facility demand-faulting in future.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/reg.h | 3 +-
arch/powerpc/kvm/book3s_hv.c | 35 ++++++++++++--------
arch/powerpc/kvm/book3s_hv_tm.c | 57 +++++++++++++++++++++------------
3 files changed, 61 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index be85cf156a1f..e9d27265253b 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -415,6 +415,7 @@
#define FSCR_TAR __MASK(FSCR_TAR_LG)
#define FSCR_EBB __MASK(FSCR_EBB_LG)
#define FSCR_DSCR __MASK(FSCR_DSCR_LG)
+#define FSCR_INTR_CAUSE (ASM_CONST(0xFF) << 56) /* interrupt cause */
#define SPRN_HFSCR 0xbe /* HV=1 Facility Status & Control Register */
#define HFSCR_PREFIX __MASK(FSCR_PREFIX_LG)
#define HFSCR_MSGP __MASK(FSCR_MSGP_LG)
@@ -426,7 +427,7 @@
#define HFSCR_DSCR __MASK(FSCR_DSCR_LG)
#define HFSCR_VECVSX __MASK(FSCR_VECVSX_LG)
#define HFSCR_FP __MASK(FSCR_FP_LG)
-#define HFSCR_INTR_CAUSE (ASM_CONST(0xFF) << 56) /* interrupt cause */
+#define HFSCR_INTR_CAUSE FSCR_INTR_CAUSE
#define SPRN_TAR 0x32f /* Target Address Register */
#define SPRN_LPCR 0x13E /* LPAR Control Register */
#define LPCR_VPM0 ASM_CONST(0x8000000000000000)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index ce7ff12cfc03..adac1a6431a0 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1682,6 +1682,21 @@ XXX benchmark guest exits
r = RESUME_GUEST;
}
break;
+
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ case BOOK3S_INTERRUPT_HV_SOFTPATCH:
+ /*
+ * This occurs for various TM-related instructions that
+ * we need to emulate on POWER9 DD2.2. We have already
+ * handled the cases where the guest was in real-suspend
+ * mode and was transitioning to transactional state.
+ */
+ r = kvmhv_p9_tm_emulation(vcpu);
+ if (r != -1)
+ break;
+ fallthrough; /* go to facility unavailable handler */
+#endif
+
/*
* This occurs if the guest (kernel or userspace), does something that
* is prohibited by HFSCR.
@@ -1700,18 +1715,6 @@ XXX benchmark guest exits
}
break;
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
- case BOOK3S_INTERRUPT_HV_SOFTPATCH:
- /*
- * This occurs for various TM-related instructions that
- * we need to emulate on POWER9 DD2.2. We have already
- * handled the cases where the guest was in real-suspend
- * mode and was transitioning to transactional state.
- */
- r = kvmhv_p9_tm_emulation(vcpu);
- break;
-#endif
-
case BOOK3S_INTERRUPT_HV_RM_HARD:
r = RESUME_PASSTHROUGH;
break;
@@ -1814,9 +1817,15 @@ static int kvmppc_handle_nested_exit(struct kvm_vcpu *vcpu)
* mode and was transitioning to transactional state.
*/
r = kvmhv_p9_tm_emulation(vcpu);
- break;
+ if (r != -1)
+ break;
+ fallthrough; /* go to facility unavailable handler */
#endif
+ case BOOK3S_INTERRUPT_H_FAC_UNAVAIL:
+ r = RESUME_HOST;
+ break;
+
case BOOK3S_INTERRUPT_HV_RM_HARD:
vcpu->arch.trap = 0;
r = RESUME_GUEST;
diff --git a/arch/powerpc/kvm/book3s_hv_tm.c b/arch/powerpc/kvm/book3s_hv_tm.c
index cc90b8b82329..e4fd4a9dee08 100644
--- a/arch/powerpc/kvm/book3s_hv_tm.c
+++ b/arch/powerpc/kvm/book3s_hv_tm.c
@@ -74,19 +74,23 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
case PPC_INST_RFEBB:
if ((msr & MSR_PR) && (vcpu->arch.vcore->pcr & PCR_ARCH_206)) {
/* generate an illegal instruction interrupt */
+ vcpu->arch.regs.nip -= 4;
kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
return RESUME_GUEST;
}
/* check EBB facility is available */
if (!(vcpu->arch.hfscr & HFSCR_EBB)) {
- /* generate an illegal instruction interrupt */
- kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
- return RESUME_GUEST;
+ vcpu->arch.regs.nip -= 4;
+ vcpu->arch.hfscr &= ~HFSCR_INTR_CAUSE;
+ vcpu->arch.hfscr |= (u64)FSCR_EBB_LG << 56;
+ vcpu->arch.trap = BOOK3S_INTERRUPT_H_FAC_UNAVAIL;
+ return -1; /* rerun host interrupt handler */
}
if ((msr & MSR_PR) && !(vcpu->arch.fscr & FSCR_EBB)) {
/* generate a facility unavailable interrupt */
- vcpu->arch.fscr = (vcpu->arch.fscr & ~(0xffull << 56)) |
- ((u64)FSCR_EBB_LG << 56);
+ vcpu->arch.regs.nip -= 4;
+ vcpu->arch.fscr &= ~FSCR_INTR_CAUSE;
+ vcpu->arch.fscr |= (u64)FSCR_EBB_LG << 56;
kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_FAC_UNAVAIL);
return RESUME_GUEST;
}
@@ -123,19 +127,23 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
/* check for PR=1 and arch 2.06 bit set in PCR */
if ((msr & MSR_PR) && (vcpu->arch.vcore->pcr & PCR_ARCH_206)) {
/* generate an illegal instruction interrupt */
+ vcpu->arch.regs.nip -= 4;
kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
return RESUME_GUEST;
}
/* check for TM disabled in the HFSCR or MSR */
if (!(vcpu->arch.hfscr & HFSCR_TM)) {
- /* generate an illegal instruction interrupt */
- kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
- return RESUME_GUEST;
+ vcpu->arch.regs.nip -= 4;
+ vcpu->arch.hfscr &= ~HFSCR_INTR_CAUSE;
+ vcpu->arch.hfscr |= (u64)FSCR_TM_LG << 56;
+ vcpu->arch.trap = BOOK3S_INTERRUPT_H_FAC_UNAVAIL;
+ return -1; /* rerun host interrupt handler */
}
if (!(msr & MSR_TM)) {
/* generate a facility unavailable interrupt */
- vcpu->arch.fscr = (vcpu->arch.fscr & ~(0xffull << 56)) |
- ((u64)FSCR_TM_LG << 56);
+ vcpu->arch.regs.nip -= 4;
+ vcpu->arch.fscr &= ~FSCR_INTR_CAUSE;
+ vcpu->arch.fscr |= (u64)FSCR_TM_LG << 56;
kvmppc_book3s_queue_irqprio(vcpu,
BOOK3S_INTERRUPT_FAC_UNAVAIL);
return RESUME_GUEST;
@@ -158,20 +166,24 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
case (PPC_INST_TRECLAIM & PO_XOP_OPCODE_MASK):
/* check for TM disabled in the HFSCR or MSR */
if (!(vcpu->arch.hfscr & HFSCR_TM)) {
- /* generate an illegal instruction interrupt */
- kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
- return RESUME_GUEST;
+ vcpu->arch.regs.nip -= 4;
+ vcpu->arch.hfscr &= ~HFSCR_INTR_CAUSE;
+ vcpu->arch.hfscr |= (u64)FSCR_TM_LG << 56;
+ vcpu->arch.trap = BOOK3S_INTERRUPT_H_FAC_UNAVAIL;
+ return -1; /* rerun host interrupt handler */
}
if (!(msr & MSR_TM)) {
/* generate a facility unavailable interrupt */
- vcpu->arch.fscr = (vcpu->arch.fscr & ~(0xffull << 56)) |
- ((u64)FSCR_TM_LG << 56);
+ vcpu->arch.regs.nip -= 4;
+ vcpu->arch.fscr &= ~FSCR_INTR_CAUSE;
+ vcpu->arch.fscr |= (u64)FSCR_TM_LG << 56;
kvmppc_book3s_queue_irqprio(vcpu,
BOOK3S_INTERRUPT_FAC_UNAVAIL);
return RESUME_GUEST;
}
/* If no transaction active, generate TM bad thing */
if (!MSR_TM_ACTIVE(msr)) {
+ vcpu->arch.regs.nip -= 4;
kvmppc_core_queue_program(vcpu, SRR1_PROGTM);
return RESUME_GUEST;
}
@@ -196,20 +208,24 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
/* XXX do we need to check for PR=0 here? */
/* check for TM disabled in the HFSCR or MSR */
if (!(vcpu->arch.hfscr & HFSCR_TM)) {
- /* generate an illegal instruction interrupt */
- kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
- return RESUME_GUEST;
+ vcpu->arch.regs.nip -= 4;
+ vcpu->arch.hfscr &= ~HFSCR_INTR_CAUSE;
+ vcpu->arch.hfscr |= (u64)FSCR_TM_LG << 56;
+ vcpu->arch.trap = BOOK3S_INTERRUPT_H_FAC_UNAVAIL;
+ return -1; /* rerun host interrupt handler */
}
if (!(msr & MSR_TM)) {
/* generate a facility unavailable interrupt */
- vcpu->arch.fscr = (vcpu->arch.fscr & ~(0xffull << 56)) |
- ((u64)FSCR_TM_LG << 56);
+ vcpu->arch.regs.nip -= 4;
+ vcpu->arch.fscr &= ~FSCR_INTR_CAUSE;
+ vcpu->arch.fscr |= (u64)FSCR_TM_LG << 56;
kvmppc_book3s_queue_irqprio(vcpu,
BOOK3S_INTERRUPT_FAC_UNAVAIL);
return RESUME_GUEST;
}
/* If transaction active or TEXASR[FS] = 0, bad thing */
if (MSR_TM_ACTIVE(msr) || !(vcpu->arch.texasr & TEXASR_FS)) {
+ vcpu->arch.regs.nip -= 4;
kvmppc_core_queue_program(vcpu, SRR1_PROGTM);
return RESUME_GUEST;
}
@@ -224,6 +240,7 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
}
/* What should we do here? We didn't recognize the instruction */
+ vcpu->arch.regs.nip -= 4;
kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
pr_warn_ratelimited("Unrecognized TM-related instruction %#x for emulation", instr);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 03/55] KVM: PPC: Book3S HV: Sanitise vcpu registers in nested path
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
From: Fabiano Rosas <farosas@linux.ibm.com>
As one of the arguments of the H_ENTER_NESTED hypercall, the nested
hypervisor (L1) prepares a structure containing the values of various
hypervisor-privileged registers with which it wants the nested guest
(L2) to run. Since the nested HV runs in supervisor mode it needs the
host to write to these registers.
To stop a nested HV manipulating this mechanism and using a nested
guest as a proxy to access a facility that has been made unavailable
to it, we have a routine that sanitises the values of the HV registers
before copying them into the nested guest's vcpu struct.
However, when coming out of the guest the values are copied as they
were back into L1 memory, which means that any sanitisation we did
during guest entry will be exposed to L1 after H_ENTER_NESTED returns.
This patch alters this sanitisation to have effect on the vcpu->arch
registers directly before entering and after exiting the guest,
leaving the structure that is copied back into L1 unchanged (except
when we really want L1 to access the value, e.g the Cause bits of
HFSCR).
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv_nested.c | 100 +++++++++++++++-------------
1 file changed, 52 insertions(+), 48 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 898f942eb198..9bb0788d312c 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -104,8 +104,17 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
{
struct kvmppc_vcore *vc = vcpu->arch.vcore;
+ /*
+ * When loading the hypervisor-privileged registers to run L2,
+ * we might have used bits from L1 state to restrict what the
+ * L2 state is allowed to be. Since L1 is not allowed to read
+ * the HV registers, do not include these modifications in the
+ * return state.
+ */
+ hr->hfscr = ((~HFSCR_INTR_CAUSE & hr->hfscr) |
+ (HFSCR_INTR_CAUSE & vcpu->arch.hfscr));
+
hr->dpdes = vc->dpdes;
- hr->hfscr = vcpu->arch.hfscr;
hr->purr = vcpu->arch.purr;
hr->spurr = vcpu->arch.spurr;
hr->ic = vcpu->arch.ic;
@@ -134,49 +143,7 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
}
}
-/*
- * This can result in some L0 HV register state being leaked to an L1
- * hypervisor when the hv_guest_state is copied back to the guest after
- * being modified here.
- *
- * There is no known problem with such a leak, and in many cases these
- * register settings could be derived by the guest by observing behaviour
- * and timing, interrupts, etc., but it is an issue to consider.
- */
-static void sanitise_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
-{
- struct kvmppc_vcore *vc = vcpu->arch.vcore;
- u64 mask;
-
- /*
- * Don't let L1 change LPCR bits for the L2 except these:
- */
- mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
- LPCR_LPES | LPCR_MER;
-
- /*
- * Additional filtering is required depending on hardware
- * and configuration.
- */
- hr->lpcr = kvmppc_filter_lpcr_hv(vcpu->kvm,
- (vc->lpcr & ~mask) | (hr->lpcr & mask));
-
- /*
- * Don't let L1 enable features for L2 which we've disabled for L1,
- * but preserve the interrupt cause field.
- */
- hr->hfscr &= (HFSCR_INTR_CAUSE | vcpu->arch.hfscr);
-
- /* Don't let data address watchpoint match in hypervisor state */
- hr->dawrx0 &= ~DAWRX_HYP;
- hr->dawrx1 &= ~DAWRX_HYP;
-
- /* Don't let completed instruction address breakpt match in HV state */
- if ((hr->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
- hr->ciabr &= ~CIABR_PRIV;
-}
-
-static void restore_hv_regs(struct kvm_vcpu *vcpu, struct hv_guest_state *hr)
+static void restore_hv_regs(struct kvm_vcpu *vcpu, const struct hv_guest_state *hr)
{
struct kvmppc_vcore *vc = vcpu->arch.vcore;
@@ -288,6 +255,43 @@ static int kvmhv_write_guest_state_and_regs(struct kvm_vcpu *vcpu,
sizeof(struct pt_regs));
}
+static void load_l2_hv_regs(struct kvm_vcpu *vcpu,
+ const struct hv_guest_state *l2_hv,
+ const struct hv_guest_state *l1_hv, u64 *lpcr)
+{
+ struct kvmppc_vcore *vc = vcpu->arch.vcore;
+ u64 mask;
+
+ restore_hv_regs(vcpu, l2_hv);
+
+ /*
+ * Don't let L1 change LPCR bits for the L2 except these:
+ */
+ mask = LPCR_DPFD | LPCR_ILE | LPCR_TC | LPCR_AIL | LPCR_LD |
+ LPCR_LPES | LPCR_MER;
+
+ /*
+ * Additional filtering is required depending on hardware
+ * and configuration.
+ */
+ *lpcr = kvmppc_filter_lpcr_hv(vcpu->kvm,
+ (vc->lpcr & ~mask) | (*lpcr & mask));
+
+ /*
+ * Don't let L1 enable features for L2 which we've disabled for L1,
+ * but preserve the interrupt cause field.
+ */
+ vcpu->arch.hfscr = l2_hv->hfscr & (HFSCR_INTR_CAUSE | l1_hv->hfscr);
+
+ /* Don't let data address watchpoint match in hypervisor state */
+ vcpu->arch.dawrx0 = l2_hv->dawrx0 & ~DAWRX_HYP;
+ vcpu->arch.dawrx1 = l2_hv->dawrx1 & ~DAWRX_HYP;
+
+ /* Don't let completed instruction address breakpt match in HV state */
+ if ((l2_hv->ciabr & CIABR_PRIV) == CIABR_PRIV_HYPER)
+ vcpu->arch.ciabr = l2_hv->ciabr & ~CIABR_PRIV;
+}
+
long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
{
long int err, r;
@@ -296,7 +300,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
struct hv_guest_state l2_hv = {0}, saved_l1_hv;
struct kvmppc_vcore *vc = vcpu->arch.vcore;
u64 hv_ptr, regs_ptr;
- u64 hdec_exp;
+ u64 hdec_exp, lpcr;
s64 delta_purr, delta_spurr, delta_ic, delta_vtb;
if (vcpu->kvm->arch.l1_ptcr == 0)
@@ -369,8 +373,8 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
/* Guest must always run with ME enabled, HV disabled. */
vcpu->arch.shregs.msr = (vcpu->arch.regs.msr | MSR_ME) & ~MSR_HV;
- sanitise_hv_regs(vcpu, &l2_hv);
- restore_hv_regs(vcpu, &l2_hv);
+ lpcr = l2_hv.lpcr;
+ load_l2_hv_regs(vcpu, &l2_hv, &saved_l1_hv, &lpcr);
vcpu->arch.ret = RESUME_GUEST;
vcpu->arch.trap = 0;
@@ -380,7 +384,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
r = RESUME_HOST;
break;
}
- r = kvmhv_run_single_vcpu(vcpu, hdec_exp, l2_hv.lpcr);
+ r = kvmhv_run_single_vcpu(vcpu, hdec_exp, lpcr);
} while (is_kvmppc_resume_guest(r));
/* save L2 state for return */
--
2.23.0
^ permalink raw reply related
* [PATCH v1 04/55] KVM: PPC: Book3S HV: Stop forwarding all HFUs to L1
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
From: Fabiano Rosas <farosas@linux.ibm.com>
If the nested hypervisor has no access to a facility because it has
been disabled by the host, it should also not be able to see the
Hypervisor Facility Unavailable that arises from one of its guests
trying to access the facility.
This patch turns a HFU that happened in L2 into a Hypervisor Emulation
Assistance interrupt and forwards it to L1 for handling. The ones that
happened because L1 explicitly disabled the facility for L2 are still
let through, along with the corresponding Cause bits in the HFSCR.
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv_nested.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
index 9bb0788d312c..983628ed4376 100644
--- a/arch/powerpc/kvm/book3s_hv_nested.c
+++ b/arch/powerpc/kvm/book3s_hv_nested.c
@@ -99,7 +99,7 @@ static void byteswap_hv_regs(struct hv_guest_state *hr)
hr->dawrx1 = swab64(hr->dawrx1);
}
-static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
+static void save_hv_return_state(struct kvm_vcpu *vcpu,
struct hv_guest_state *hr)
{
struct kvmppc_vcore *vc = vcpu->arch.vcore;
@@ -128,7 +128,7 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
hr->pidr = vcpu->arch.pid;
hr->cfar = vcpu->arch.cfar;
hr->ppr = vcpu->arch.ppr;
- switch (trap) {
+ switch (vcpu->arch.trap) {
case BOOK3S_INTERRUPT_H_DATA_STORAGE:
hr->hdar = vcpu->arch.fault_dar;
hr->hdsisr = vcpu->arch.fault_dsisr;
@@ -137,6 +137,27 @@ static void save_hv_return_state(struct kvm_vcpu *vcpu, int trap,
case BOOK3S_INTERRUPT_H_INST_STORAGE:
hr->asdr = vcpu->arch.fault_gpa;
break;
+ case BOOK3S_INTERRUPT_H_FAC_UNAVAIL:
+ {
+ u8 cause = vcpu->arch.hfscr >> 56;
+
+ WARN_ON_ONCE(cause >= BITS_PER_LONG);
+
+ if (!(hr->hfscr & (1UL << cause)))
+ break;
+
+ /*
+ * We have disabled this facility, so it does not
+ * exist from L1's perspective. Turn it into a HEAI.
+ */
+ vcpu->arch.trap = BOOK3S_INTERRUPT_H_EMUL_ASSIST;
+ kvmppc_load_last_inst(vcpu, INST_GENERIC, &vcpu->arch.emul_inst);
+
+ /* Don't leak the cause field */
+ hr->hfscr &= ~HFSCR_INTR_CAUSE;
+
+ fallthrough;
+ }
case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
hr->heir = vcpu->arch.emul_inst;
break;
@@ -394,7 +415,7 @@ long kvmhv_enter_nested_guest(struct kvm_vcpu *vcpu)
delta_spurr = vcpu->arch.spurr - l2_hv.spurr;
delta_ic = vcpu->arch.ic - l2_hv.ic;
delta_vtb = vc->vtb - l2_hv.vtb;
- save_hv_return_state(vcpu, vcpu->arch.trap, &l2_hv);
+ save_hv_return_state(vcpu, &l2_hv);
/* restore L1 state */
vcpu->arch.nested = NULL;
--
2.23.0
^ permalink raw reply related
* [PATCH v1 05/55] KVM: PPC: Book3S HV Nested: Reflect guest PMU in-use to L0 when guest SPRs are live
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
After the L1 saves its PMU SPRs but before loading the L2's PMU SPRs,
switch the pmcregs_in_use field in the L1 lppaca to the value advertised
by the L2 in its VPA. On the way out of the L2, set it back after saving
the L2 PMU registers (if they were in-use).
This transfers the PMU liveness indication between the L1 and L2 at the
points where the registers are not live.
This fixes the nested HV bug for which a workaround was added to the L0
HV by commit 63279eeb7f93a ("KVM: PPC: Book3S HV: Always save guest pmu
for guest capable of nesting"), which explains the problem in detail.
That workaround is no longer required for guests that include this bug
fix.
Fixes: 360cae313702 ("KVM: PPC: Book3S HV: Nested guest entry via hypercall")
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/pmc.h | 7 +++++++
arch/powerpc/kvm/book3s_hv.c | 20 ++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/arch/powerpc/include/asm/pmc.h b/arch/powerpc/include/asm/pmc.h
index c6bbe9778d3c..3c09109e708e 100644
--- a/arch/powerpc/include/asm/pmc.h
+++ b/arch/powerpc/include/asm/pmc.h
@@ -34,6 +34,13 @@ static inline void ppc_set_pmu_inuse(int inuse)
#endif
}
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+static inline int ppc_get_pmu_inuse(void)
+{
+ return get_paca()->pmcregs_in_use;
+}
+#endif
+
extern void power4_enable_pmcs(void);
#else /* CONFIG_PPC64 */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index adac1a6431a0..c743020837e7 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -59,6 +59,7 @@
#include <asm/kvm_book3s.h>
#include <asm/mmu_context.h>
#include <asm/lppaca.h>
+#include <asm/pmc.h>
#include <asm/processor.h>
#include <asm/cputhreads.h>
#include <asm/page.h>
@@ -3864,6 +3865,18 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
cpu_has_feature(CPU_FTR_P9_TM_HV_ASSIST))
kvmppc_restore_tm_hv(vcpu, vcpu->arch.shregs.msr, true);
+#ifdef CONFIG_PPC_PSERIES
+ if (kvmhv_on_pseries()) {
+ barrier();
+ if (vcpu->arch.vpa.pinned_addr) {
+ struct lppaca *lp = vcpu->arch.vpa.pinned_addr;
+ get_lppaca()->pmcregs_in_use = lp->pmcregs_in_use;
+ } else {
+ get_lppaca()->pmcregs_in_use = 1;
+ }
+ barrier();
+ }
+#endif
kvmhv_load_guest_pmu(vcpu);
msr_check_and_set(MSR_FP | MSR_VEC | MSR_VSX);
@@ -3998,6 +4011,13 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
save_pmu |= nesting_enabled(vcpu->kvm);
kvmhv_save_guest_pmu(vcpu, save_pmu);
+#ifdef CONFIG_PPC_PSERIES
+ if (kvmhv_on_pseries()) {
+ barrier();
+ get_lppaca()->pmcregs_in_use = ppc_get_pmu_inuse();
+ barrier();
+ }
+#endif
vc->entry_exit_map = 0x101;
vc->in_guest = 0;
--
2.23.0
^ permalink raw reply related
* [PATCH v1 06/55] powerpc/64s: Remove WORT SPR from POWER9/10
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
This register is not architected and not implemented in POWER9 or 10,
it just reads back zeroes for compatibility.
-78 cycles (9255) cycles POWER9 virt-mode NULL hcall
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 3 ---
arch/powerpc/platforms/powernv/idle.c | 2 --
2 files changed, 5 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index c743020837e7..905bf29940ea 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3740,7 +3740,6 @@ static void load_spr_state(struct kvm_vcpu *vcpu)
mtspr(SPRN_EBBHR, vcpu->arch.ebbhr);
mtspr(SPRN_EBBRR, vcpu->arch.ebbrr);
mtspr(SPRN_BESCR, vcpu->arch.bescr);
- mtspr(SPRN_WORT, vcpu->arch.wort);
mtspr(SPRN_TIDR, vcpu->arch.tid);
mtspr(SPRN_AMR, vcpu->arch.amr);
mtspr(SPRN_UAMOR, vcpu->arch.uamor);
@@ -3767,7 +3766,6 @@ static void store_spr_state(struct kvm_vcpu *vcpu)
vcpu->arch.ebbhr = mfspr(SPRN_EBBHR);
vcpu->arch.ebbrr = mfspr(SPRN_EBBRR);
vcpu->arch.bescr = mfspr(SPRN_BESCR);
- vcpu->arch.wort = mfspr(SPRN_WORT);
vcpu->arch.tid = mfspr(SPRN_TIDR);
vcpu->arch.amr = mfspr(SPRN_AMR);
vcpu->arch.uamor = mfspr(SPRN_UAMOR);
@@ -3799,7 +3797,6 @@ static void restore_p9_host_os_sprs(struct kvm_vcpu *vcpu,
struct p9_host_os_sprs *host_os_sprs)
{
mtspr(SPRN_PSPB, 0);
- mtspr(SPRN_WORT, 0);
mtspr(SPRN_UAMOR, 0);
mtspr(SPRN_DSCR, host_os_sprs->dscr);
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 1e908536890b..df19e2ff9d3c 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -667,7 +667,6 @@ static unsigned long power9_idle_stop(unsigned long psscr)
sprs.purr = mfspr(SPRN_PURR);
sprs.spurr = mfspr(SPRN_SPURR);
sprs.dscr = mfspr(SPRN_DSCR);
- sprs.wort = mfspr(SPRN_WORT);
sprs.ciabr = mfspr(SPRN_CIABR);
sprs.mmcra = mfspr(SPRN_MMCRA);
@@ -785,7 +784,6 @@ static unsigned long power9_idle_stop(unsigned long psscr)
mtspr(SPRN_PURR, sprs.purr);
mtspr(SPRN_SPURR, sprs.spurr);
mtspr(SPRN_DSCR, sprs.dscr);
- mtspr(SPRN_WORT, sprs.wort);
mtspr(SPRN_CIABR, sprs.ciabr);
mtspr(SPRN_MMCRA, sprs.mmcra);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 07/55] KMV: PPC: Book3S HV P9: Use set_dec to set decrementer to host
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
The host Linux timer code arms the decrementer with the value
'decrementers_next_tb - current_tb' using set_dec(), which stores
val - 1 on Book3S-64, which is not quite the same as what KVM does
to re-arm the host decrementer when exiting the guest.
This shouldn't be a significant change, but it makes the logic match
and avoids this small extra change being brought into the next patch.
Suggested-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 905bf29940ea..7020cbbf3aa1 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4019,7 +4019,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
vc->entry_exit_map = 0x101;
vc->in_guest = 0;
- mtspr(SPRN_DEC, local_paca->kvm_hstate.dec_expires - mftb());
+ set_dec(local_paca->kvm_hstate.dec_expires - mftb());
/* We may have raced with new irq work */
if (test_irq_work_pending())
set_dec(1);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 08/55] KVM: PPC: Book3S HV P9: Use host timer accounting to avoid decrementer read
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
There is no need to save away the host DEC value, as it is derived
from the host timer subsystem which maintains the next timer time,
so it can be restored from there.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/time.h | 5 +++++
arch/powerpc/kernel/time.c | 1 +
arch/powerpc/kvm/book3s_hv.c | 14 +++++++-------
3 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 8c2c3dd4ddba..fd09b4797fd7 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -111,6 +111,11 @@ static inline unsigned long test_irq_work_pending(void)
DECLARE_PER_CPU(u64, decrementers_next_tb);
+static inline u64 timer_get_next_tb(void)
+{
+ return __this_cpu_read(decrementers_next_tb);
+}
+
/* Convert timebase ticks to nanoseconds */
unsigned long long tb_to_ns(unsigned long long tb_ticks);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index e45ce427bffb..01df89918aa4 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -108,6 +108,7 @@ struct clock_event_device decrementer_clockevent = {
EXPORT_SYMBOL(decrementer_clockevent);
DEFINE_PER_CPU(u64, decrementers_next_tb);
+EXPORT_SYMBOL_GPL(decrementers_next_tb);
static DEFINE_PER_CPU(struct clock_event_device, decrementers);
#define XSEC_PER_SEC (1024*1024)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 7020cbbf3aa1..82976f734bd1 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3829,18 +3829,17 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
struct kvmppc_vcore *vc = vcpu->arch.vcore;
struct p9_host_os_sprs host_os_sprs;
s64 dec;
- u64 tb;
+ u64 tb, next_timer;
int trap, save_pmu;
WARN_ON_ONCE(vcpu->arch.ceded);
- dec = mfspr(SPRN_DEC);
tb = mftb();
- if (dec < 0)
+ next_timer = timer_get_next_tb();
+ if (tb >= next_timer)
return BOOK3S_INTERRUPT_HV_DECREMENTER;
- local_paca->kvm_hstate.dec_expires = dec + tb;
- if (local_paca->kvm_hstate.dec_expires < time_limit)
- time_limit = local_paca->kvm_hstate.dec_expires;
+ if (next_timer < time_limit)
+ time_limit = next_timer;
save_p9_host_os_sprs(&host_os_sprs);
@@ -4019,7 +4018,8 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
vc->entry_exit_map = 0x101;
vc->in_guest = 0;
- set_dec(local_paca->kvm_hstate.dec_expires - mftb());
+ next_timer = timer_get_next_tb();
+ set_dec(next_timer - mftb());
/* We may have raced with new irq work */
if (test_irq_work_pending())
set_dec(1);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 09/55] KVM: PPC: Book3S HV P9: Use large decrementer for HDEC
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: Alexey Kardashevskiy, linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
On processors that don't suppress the HDEC exceptions when LPCR[HDICE]=0,
this could help reduce needless guest exits due to leftover exceptions on
entering the guest.
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/time.h | 2 ++
arch/powerpc/kernel/time.c | 1 +
arch/powerpc/kvm/book3s_hv_p9_entry.c | 3 ++-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index fd09b4797fd7..69b6be617772 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -18,6 +18,8 @@
#include <asm/vdso/timebase.h>
/* time.c */
+extern u64 decrementer_max;
+
extern unsigned long tb_ticks_per_jiffy;
extern unsigned long tb_ticks_per_usec;
extern unsigned long tb_ticks_per_sec;
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 01df89918aa4..72d872b49167 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -89,6 +89,7 @@ static struct clocksource clocksource_timebase = {
#define DECREMENTER_DEFAULT_MAX 0x7FFFFFFF
u64 decrementer_max = DECREMENTER_DEFAULT_MAX;
+EXPORT_SYMBOL_GPL(decrementer_max); /* for KVM HDEC */
static int decrementer_set_next_event(unsigned long evt,
struct clock_event_device *dev);
diff --git a/arch/powerpc/kvm/book3s_hv_p9_entry.c b/arch/powerpc/kvm/book3s_hv_p9_entry.c
index 961b3d70483c..0ff9ddb5e7ca 100644
--- a/arch/powerpc/kvm/book3s_hv_p9_entry.c
+++ b/arch/powerpc/kvm/book3s_hv_p9_entry.c
@@ -504,7 +504,8 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
vc->tb_offset_applied = 0;
}
- mtspr(SPRN_HDEC, 0x7fffffff);
+ /* HDEC must be at least as large as DEC, so decrementer_max fits */
+ mtspr(SPRN_HDEC, decrementer_max);
save_clear_guest_mmu(kvm, vcpu);
switch_mmu_to_host(kvm, host_pidr);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 10/55] KVM: PPC: Book3S HV P9: Reduce mftb per guest entry/exit
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
mftb is serialising (dispatch next-to-complete) so it is heavy weight
for a mfspr. Avoid reading it multiple times in the entry or exit paths.
A small number of cycles delay to timers is tolerable.
-118 cycles (9137) POWER9 virt-mode NULL hcall
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 4 ++--
arch/powerpc/kvm/book3s_hv_p9_entry.c | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 82976f734bd1..6e6cfb10e9bb 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3896,7 +3896,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
*
* XXX: Another day's problem.
*/
- mtspr(SPRN_DEC, vcpu->arch.dec_expires - mftb());
+ mtspr(SPRN_DEC, vcpu->arch.dec_expires - tb);
if (kvmhv_on_pseries()) {
/*
@@ -4019,7 +4019,7 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
vc->in_guest = 0;
next_timer = timer_get_next_tb();
- set_dec(next_timer - mftb());
+ set_dec(next_timer - tb);
/* We may have raced with new irq work */
if (test_irq_work_pending())
set_dec(1);
diff --git a/arch/powerpc/kvm/book3s_hv_p9_entry.c b/arch/powerpc/kvm/book3s_hv_p9_entry.c
index 0ff9ddb5e7ca..bd8cf0a65ce8 100644
--- a/arch/powerpc/kvm/book3s_hv_p9_entry.c
+++ b/arch/powerpc/kvm/book3s_hv_p9_entry.c
@@ -203,7 +203,8 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
unsigned long host_dawr1;
unsigned long host_dawrx1;
- hdec = time_limit - mftb();
+ tb = mftb();
+ hdec = time_limit - tb;
if (hdec < 0)
return BOOK3S_INTERRUPT_HV_DECREMENTER;
@@ -215,7 +216,7 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
vcpu->arch.ceded = 0;
if (vc->tb_offset) {
- u64 new_tb = mftb() + vc->tb_offset;
+ u64 new_tb = tb + vc->tb_offset;
mtspr(SPRN_TBU40, new_tb);
tb = mftb();
if ((tb & 0xffffff) < (new_tb & 0xffffff))
--
2.23.0
^ permalink raw reply related
* [PATCH v1 11/55] powerpc/time: add API for KVM to re-arm the host timer/decrementer
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
Rather than have KVM look up the host timer and fiddle with the
irq-work internal details, have the powerpc/time.c code provide a
function for KVM to re-arm the Linux timer code when exiting a
guest.
This is implementation has an improvement over existing code of
marking a decrementer interrupt as soft-pending if a timer has
expired, rather than setting DEC to a -ve value, which tended to
cause host timers to take two interrupts (first hdec to exit the
guest, then the immediate dec).
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/include/asm/time.h | 16 +++-------
arch/powerpc/kernel/time.c | 52 +++++++++++++++++++++++++++------
arch/powerpc/kvm/book3s_hv.c | 7 ++---
3 files changed, 49 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index 69b6be617772..924b2157882f 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -99,18 +99,6 @@ extern void div128_by_32(u64 dividend_high, u64 dividend_low,
extern void secondary_cpu_time_init(void);
extern void __init time_init(void);
-#ifdef CONFIG_PPC64
-static inline unsigned long test_irq_work_pending(void)
-{
- unsigned long x;
-
- asm volatile("lbz %0,%1(13)"
- : "=r" (x)
- : "i" (offsetof(struct paca_struct, irq_work_pending)));
- return x;
-}
-#endif
-
DECLARE_PER_CPU(u64, decrementers_next_tb);
static inline u64 timer_get_next_tb(void)
@@ -118,6 +106,10 @@ static inline u64 timer_get_next_tb(void)
return __this_cpu_read(decrementers_next_tb);
}
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+void timer_rearm_host_dec(u64 now);
+#endif
+
/* Convert timebase ticks to nanoseconds */
unsigned long long tb_to_ns(unsigned long long tb_ticks);
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 72d872b49167..016828b7401b 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -499,6 +499,16 @@ EXPORT_SYMBOL(profile_pc);
* 64-bit uses a byte in the PACA, 32-bit uses a per-cpu variable...
*/
#ifdef CONFIG_PPC64
+static inline unsigned long test_irq_work_pending(void)
+{
+ unsigned long x;
+
+ asm volatile("lbz %0,%1(13)"
+ : "=r" (x)
+ : "i" (offsetof(struct paca_struct, irq_work_pending)));
+ return x;
+}
+
static inline void set_irq_work_pending_flag(void)
{
asm volatile("stb %0,%1(13)" : :
@@ -542,13 +552,44 @@ void arch_irq_work_raise(void)
preempt_enable();
}
+static void set_dec_or_work(u64 val)
+{
+ set_dec(val);
+ /* We may have raced with new irq work */
+ if (unlikely(test_irq_work_pending()))
+ set_dec(1);
+}
+
#else /* CONFIG_IRQ_WORK */
#define test_irq_work_pending() 0
#define clear_irq_work_pending()
+static void set_dec_or_work(u64 val)
+{
+ set_dec(val);
+}
#endif /* CONFIG_IRQ_WORK */
+#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+void timer_rearm_host_dec(u64 now)
+{
+ u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
+
+ WARN_ON_ONCE(!arch_irqs_disabled());
+ WARN_ON_ONCE(mfmsr() & MSR_EE);
+
+ if (now >= *next_tb) {
+ local_paca->irq_happened |= PACA_IRQ_DEC;
+ } else {
+ now = *next_tb - now;
+ if (now <= decrementer_max)
+ set_dec_or_work(now);
+ }
+}
+EXPORT_SYMBOL_GPL(timer_rearm_host_dec);
+#endif
+
/*
* timer_interrupt - gets called when the decrementer overflows,
* with interrupts disabled.
@@ -609,10 +650,7 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(timer_interrupt)
} else {
now = *next_tb - now;
if (now <= decrementer_max)
- set_dec(now);
- /* We may have raced with new irq work */
- if (test_irq_work_pending())
- set_dec(1);
+ set_dec_or_work(now);
__this_cpu_inc(irq_stat.timer_irqs_others);
}
@@ -854,11 +892,7 @@ static int decrementer_set_next_event(unsigned long evt,
struct clock_event_device *dev)
{
__this_cpu_write(decrementers_next_tb, get_tb() + evt);
- set_dec(evt);
-
- /* We may have raced with new irq work */
- if (test_irq_work_pending())
- set_dec(1);
+ set_dec_or_work(evt);
return 0;
}
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 6e6cfb10e9bb..0cef578930f9 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4018,11 +4018,8 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
vc->entry_exit_map = 0x101;
vc->in_guest = 0;
- next_timer = timer_get_next_tb();
- set_dec(next_timer - tb);
- /* We may have raced with new irq work */
- if (test_irq_work_pending())
- set_dec(1);
+ timer_rearm_host_dec(tb);
+
mtspr(SPRN_SPRG_VDSO_WRITE, local_paca->sprg_vdso);
kvmhv_load_host_pmu();
--
2.23.0
^ permalink raw reply related
* [PATCH v1 12/55] KVM: PPC: Book3S HV: POWER10 enable HAIL when running radix guests
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
HV interrupts may be taken with the MMU enabled when radix guests are
running. Enable LPCR[HAIL] on ISA v3.1 processors for radix guests.
Make this depend on the host LPCR[HAIL] being enabled. Currently that is
always enabled, but having this test means any issue that might require
LPCR[HAIL] to be disabled in the host will not have to be duplicated in
KVM.
-1380 cycles on P10 NULL hcall entry+exit
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 0cef578930f9..e7f8cc04944b 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -5004,6 +5004,8 @@ static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu)
*/
int kvmppc_switch_mmu_to_hpt(struct kvm *kvm)
{
+ unsigned long lpcr, lpcr_mask;
+
if (nesting_enabled(kvm))
kvmhv_release_all_nested(kvm);
kvmppc_rmap_reset(kvm);
@@ -5013,8 +5015,13 @@ int kvmppc_switch_mmu_to_hpt(struct kvm *kvm)
kvm->arch.radix = 0;
spin_unlock(&kvm->mmu_lock);
kvmppc_free_radix(kvm);
- kvmppc_update_lpcr(kvm, LPCR_VPM1,
- LPCR_VPM1 | LPCR_UPRT | LPCR_GTSE | LPCR_HR);
+
+ lpcr = LPCR_VPM1;
+ lpcr_mask = LPCR_VPM1 | LPCR_UPRT | LPCR_GTSE | LPCR_HR;
+ if (cpu_has_feature(CPU_FTR_ARCH_31))
+ lpcr_mask |= LPCR_HAIL;
+ kvmppc_update_lpcr(kvm, lpcr, lpcr_mask);
+
return 0;
}
@@ -5024,6 +5031,7 @@ int kvmppc_switch_mmu_to_hpt(struct kvm *kvm)
*/
int kvmppc_switch_mmu_to_radix(struct kvm *kvm)
{
+ unsigned long lpcr, lpcr_mask;
int err;
err = kvmppc_init_vm_radix(kvm);
@@ -5035,8 +5043,17 @@ int kvmppc_switch_mmu_to_radix(struct kvm *kvm)
kvm->arch.radix = 1;
spin_unlock(&kvm->mmu_lock);
kvmppc_free_hpt(&kvm->arch.hpt);
- kvmppc_update_lpcr(kvm, LPCR_UPRT | LPCR_GTSE | LPCR_HR,
- LPCR_VPM1 | LPCR_UPRT | LPCR_GTSE | LPCR_HR);
+
+ lpcr = LPCR_UPRT | LPCR_GTSE | LPCR_HR;
+ lpcr_mask = LPCR_VPM1 | LPCR_UPRT | LPCR_GTSE | LPCR_HR;
+ if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+ lpcr_mask |= LPCR_HAIL;
+ if (cpu_has_feature(CPU_FTR_HVMODE) &&
+ (kvm->arch.host_lpcr & LPCR_HAIL))
+ lpcr |= LPCR_HAIL;
+ }
+ kvmppc_update_lpcr(kvm, lpcr, lpcr_mask);
+
return 0;
}
@@ -5200,6 +5217,10 @@ static int kvmppc_core_init_vm_hv(struct kvm *kvm)
kvm->arch.mmu_ready = 1;
lpcr &= ~LPCR_VPM1;
lpcr |= LPCR_UPRT | LPCR_GTSE | LPCR_HR;
+ if (cpu_has_feature(CPU_FTR_HVMODE) &&
+ cpu_has_feature(CPU_FTR_ARCH_31) &&
+ (kvm->arch.host_lpcr & LPCR_HAIL))
+ lpcr |= LPCR_HAIL;
ret = kvmppc_init_vm_radix(kvm);
if (ret) {
kvmppc_free_lpid(kvm->arch.lpid);
--
2.23.0
^ permalink raw reply related
* [PATCH v1 13/55] powerpc/64s: Keep AMOR SPR a constant ~0 at runtime
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin, Fabiano Rosas
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
This register controls supervisor SPR modifications, and as such is only
relevant for KVM. KVM always sets AMOR to ~0 on guest entry, and never
restores it coming back out to the host, so it can be kept constant and
avoid the mtSPR in KVM guest entry.
-21 cycles (9116) cycles POWER9 virt-mode NULL hcall
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/cpu_setup_power.c | 8 ++++++++
arch/powerpc/kernel/dt_cpu_ftrs.c | 2 ++
arch/powerpc/kvm/book3s_hv_p9_entry.c | 2 --
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 2 --
arch/powerpc/mm/book3s64/radix_pgtable.c | 15 ---------------
arch/powerpc/platforms/powernv/idle.c | 8 +++-----
6 files changed, 13 insertions(+), 24 deletions(-)
diff --git a/arch/powerpc/kernel/cpu_setup_power.c b/arch/powerpc/kernel/cpu_setup_power.c
index 3cca88ee96d7..a29dc8326622 100644
--- a/arch/powerpc/kernel/cpu_setup_power.c
+++ b/arch/powerpc/kernel/cpu_setup_power.c
@@ -137,6 +137,7 @@ void __setup_cpu_power7(unsigned long offset, struct cpu_spec *t)
return;
mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_PCR, PCR_MASK);
init_LPCR_ISA206(mfspr(SPRN_LPCR), LPCR_LPES1 >> LPCR_LPES_SH);
}
@@ -150,6 +151,7 @@ void __restore_cpu_power7(void)
return;
mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_PCR, PCR_MASK);
init_LPCR_ISA206(mfspr(SPRN_LPCR), LPCR_LPES1 >> LPCR_LPES_SH);
}
@@ -164,6 +166,7 @@ void __setup_cpu_power8(unsigned long offset, struct cpu_spec *t)
return;
mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_PCR, PCR_MASK);
init_LPCR_ISA206(mfspr(SPRN_LPCR) | LPCR_PECEDH, 0); /* LPES = 0 */
init_HFSCR();
@@ -184,6 +187,7 @@ void __restore_cpu_power8(void)
return;
mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_PCR, PCR_MASK);
init_LPCR_ISA206(mfspr(SPRN_LPCR) | LPCR_PECEDH, 0); /* LPES = 0 */
init_HFSCR();
@@ -202,6 +206,7 @@ void __setup_cpu_power9(unsigned long offset, struct cpu_spec *t)
mtspr(SPRN_PSSCR, 0);
mtspr(SPRN_LPID, 0);
mtspr(SPRN_PID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_PCR, PCR_MASK);
init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
@@ -223,6 +228,7 @@ void __restore_cpu_power9(void)
mtspr(SPRN_PSSCR, 0);
mtspr(SPRN_LPID, 0);
mtspr(SPRN_PID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_PCR, PCR_MASK);
init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
@@ -242,6 +248,7 @@ void __setup_cpu_power10(unsigned long offset, struct cpu_spec *t)
mtspr(SPRN_PSSCR, 0);
mtspr(SPRN_LPID, 0);
mtspr(SPRN_PID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_PCR, PCR_MASK);
init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
@@ -264,6 +271,7 @@ void __restore_cpu_power10(void)
mtspr(SPRN_PSSCR, 0);
mtspr(SPRN_LPID, 0);
mtspr(SPRN_PID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_PCR, PCR_MASK);
init_LPCR_ISA300((mfspr(SPRN_LPCR) | LPCR_PECEDH | LPCR_PECE_HVEE |\
LPCR_HVICE | LPCR_HEIC) & ~(LPCR_UPRT | LPCR_HR), 0);
diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
index af95f337e54b..38ea20fadc4a 100644
--- a/arch/powerpc/kernel/dt_cpu_ftrs.c
+++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
@@ -80,6 +80,7 @@ static void __restore_cpu_cpufeatures(void)
mtspr(SPRN_LPCR, system_registers.lpcr);
if (hv_mode) {
mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_HFSCR, system_registers.hfscr);
mtspr(SPRN_PCR, system_registers.pcr);
}
@@ -216,6 +217,7 @@ static int __init feat_enable_hv(struct dt_cpu_feature *f)
}
mtspr(SPRN_LPID, 0);
+ mtspr(SPRN_AMOR, ~0);
lpcr = mfspr(SPRN_LPCR);
lpcr &= ~LPCR_LPES0; /* HV external interrupts */
diff --git a/arch/powerpc/kvm/book3s_hv_p9_entry.c b/arch/powerpc/kvm/book3s_hv_p9_entry.c
index bd8cf0a65ce8..a7f63082b4e3 100644
--- a/arch/powerpc/kvm/book3s_hv_p9_entry.c
+++ b/arch/powerpc/kvm/book3s_hv_p9_entry.c
@@ -286,8 +286,6 @@ int kvmhv_vcpu_entry_p9(struct kvm_vcpu *vcpu, u64 time_limit, unsigned long lpc
mtspr(SPRN_SPRG2, vcpu->arch.shregs.sprg2);
mtspr(SPRN_SPRG3, vcpu->arch.shregs.sprg3);
- mtspr(SPRN_AMOR, ~0UL);
-
local_paca->kvm_hstate.in_guest = KVM_GUEST_MODE_HV_P9;
/*
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 75079397c2a5..9021052f1579 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -772,10 +772,8 @@ END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_207S)
/* Restore AMR and UAMOR, set AMOR to all 1s */
ld r5,VCPU_AMR(r4)
ld r6,VCPU_UAMOR(r4)
- li r7,-1
mtspr SPRN_AMR,r5
mtspr SPRN_UAMOR,r6
- mtspr SPRN_AMOR,r7
/* Restore state of CTRL run bit; assume 1 on entry */
lwz r5,VCPU_CTRL(r4)
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
index e50ddf129c15..5aebd70ef66a 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -572,18 +572,6 @@ void __init radix__early_init_devtree(void)
return;
}
-static void radix_init_amor(void)
-{
- /*
- * In HV mode, we init AMOR (Authority Mask Override Register) so that
- * the hypervisor and guest can setup IAMR (Instruction Authority Mask
- * Register), enable key 0 and set it to 1.
- *
- * AMOR = 0b1100 .... 0000 (Mask for key 0 is 11)
- */
- mtspr(SPRN_AMOR, (3ul << 62));
-}
-
void __init radix__early_init_mmu(void)
{
unsigned long lpcr;
@@ -644,7 +632,6 @@ void __init radix__early_init_mmu(void)
lpcr = mfspr(SPRN_LPCR);
mtspr(SPRN_LPCR, lpcr | LPCR_UPRT | LPCR_HR);
radix_init_partition_table();
- radix_init_amor();
} else {
radix_init_pseries();
}
@@ -668,8 +655,6 @@ void radix__early_init_mmu_secondary(void)
set_ptcr_when_no_uv(__pa(partition_tb) |
(PATB_SIZE_SHIFT - 12));
-
- radix_init_amor();
}
radix__switch_mmu_context(NULL, &init_mm);
diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index df19e2ff9d3c..721ac4f7e2d1 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -306,8 +306,8 @@ struct p7_sprs {
/* per thread SPRs that get lost in shallow states */
u64 amr;
u64 iamr;
- u64 amor;
u64 uamor;
+ /* amor is restored to constant ~0 */
};
static unsigned long power7_idle_insn(unsigned long type)
@@ -378,7 +378,6 @@ static unsigned long power7_idle_insn(unsigned long type)
if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
sprs.amr = mfspr(SPRN_AMR);
sprs.iamr = mfspr(SPRN_IAMR);
- sprs.amor = mfspr(SPRN_AMOR);
sprs.uamor = mfspr(SPRN_UAMOR);
}
@@ -397,7 +396,7 @@ static unsigned long power7_idle_insn(unsigned long type)
*/
mtspr(SPRN_AMR, sprs.amr);
mtspr(SPRN_IAMR, sprs.iamr);
- mtspr(SPRN_AMOR, sprs.amor);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_UAMOR, sprs.uamor);
}
}
@@ -687,7 +686,6 @@ static unsigned long power9_idle_stop(unsigned long psscr)
sprs.amr = mfspr(SPRN_AMR);
sprs.iamr = mfspr(SPRN_IAMR);
- sprs.amor = mfspr(SPRN_AMOR);
sprs.uamor = mfspr(SPRN_UAMOR);
srr1 = isa300_idle_stop_mayloss(psscr); /* go idle */
@@ -708,7 +706,7 @@ static unsigned long power9_idle_stop(unsigned long psscr)
*/
mtspr(SPRN_AMR, sprs.amr);
mtspr(SPRN_IAMR, sprs.iamr);
- mtspr(SPRN_AMOR, sprs.amor);
+ mtspr(SPRN_AMOR, ~0);
mtspr(SPRN_UAMOR, sprs.uamor);
/*
--
2.23.0
^ permalink raw reply related
* [PATCH v1 14/55] KVM: PPC: Book3S HV: Don't always save PMU for guest capable of nesting
From: Nicholas Piggin @ 2021-07-26 3:49 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, Nicholas Piggin
In-Reply-To: <20210726035036.739609-1-npiggin@gmail.com>
Revert the workaround added by commit 63279eeb7f93a ("KVM: PPC: Book3S
HV: Always save guest pmu for guest capable of nesting").
Nested capable guests running with the earlier commit ("KVM: PPC: Book3S
HV Nested: Indicate guest PMU in-use in VPA") will now indicate the PMU
in-use status of their guests, which means the parent does not need to
unconditionally save the PMU for nested capable guests.
This will cause the PMU to break for nested guests when running older
nested hypervisor guests under a kernel with this change. It's unclear
there's an easy way to avoid that, so this could wait for a release or
so for the fix to filter into stable kernels.
-134 cycles (8982) POWER9 virt-mode NULL hcall
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kvm/book3s_hv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index e7f8cc04944b..ab89db561c85 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4003,8 +4003,6 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
vcpu->arch.vpa.dirty = 1;
save_pmu = lp->pmcregs_in_use;
}
- /* Must save pmu if this guest is capable of running nested guests */
- save_pmu |= nesting_enabled(vcpu->kvm);
kvmhv_save_guest_pmu(vcpu, save_pmu);
#ifdef CONFIG_PPC_PSERIES
--
2.23.0
^ permalink raw reply related
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